From 648f02e85a0f322163a18a37c30273175ee133d9 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 4 Jun 2026 14:42:48 +0200 Subject: [PATCH 1/3] benchmark: scope & value --- tests/benchmark.py | 41 +++++++++- tests/benchmark/CMakeLists.txt | 2 + tests/benchmark/benchmark_scope.cpp | 98 +++++++++++++++++++++++ tests/benchmark/benchmark_value.cpp | 116 ++++++++++++++++++++++++++++ tests/conftest.py | 15 +++- 5 files changed, 265 insertions(+), 7 deletions(-) create mode 100644 tests/benchmark/benchmark_scope.cpp create mode 100644 tests/benchmark/benchmark_value.cpp diff --git a/tests/benchmark.py b/tests/benchmark.py index 2bc909ed5d..c4a9898a0f 100644 --- a/tests/benchmark.py +++ b/tests/benchmark.py @@ -6,7 +6,7 @@ from . import make_dsn, run -def run_benchmark(target, backend, cmake, httpserver, gbenchmark, label): +def run_benchmark(target, backend, cmake, httpserver, gbenchmark, label, runs=1): tmp_path = cmake( ["sentry_benchmark"], { @@ -19,7 +19,7 @@ def run_benchmark(target, backend, cmake, httpserver, gbenchmark, label): env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver)) benchmark_out = tmp_path / "benchmark.json" - for i in range(6): + for i in range(runs): # make sure we are isolated from previous runs shutil.rmtree(tmp_path / ".sentry-native", ignore_errors=True) @@ -31,14 +31,14 @@ def run_benchmark(target, backend, cmake, httpserver, gbenchmark, label): ) # ignore warmup run - if i > 0: + if runs == 1 or i > 0: gbenchmark(benchmark_out, label) @pytest.mark.parametrize("backend", ["inproc", "breakpad", "crashpad"]) def test_benchmark_init(backend, cmake, httpserver, gbenchmark): run_benchmark( - "init", backend, cmake, httpserver, gbenchmark, f"SDK init ({backend})" + "init", backend, cmake, httpserver, gbenchmark, f"SDK init ({backend})", 6 ) @@ -51,4 +51,37 @@ def test_benchmark_backend(backend, cmake, httpserver, gbenchmark): httpserver, gbenchmark, f"Backend startup ({backend})", + 6, + ) + + +@pytest.mark.parametrize("test_name", ["apply", "capture"]) +def test_benchmark_scope(cmake, httpserver, gbenchmark, test_name): + run_benchmark( + f"benchmark_scope_{test_name}", + "none", + cmake, + httpserver, + gbenchmark, + f"Scope ({test_name})", + ) + + +@pytest.mark.parametrize( + "test_name", + [ + "log_attributes", + "flat_object", + "nested_object", + "flat_list", + ], +) +def test_benchmark_value(cmake, httpserver, gbenchmark, test_name): + run_benchmark( + f"benchmark_value_{test_name}", + "none", + cmake, + httpserver, + gbenchmark, + f"Value ({test_name})", ) diff --git a/tests/benchmark/CMakeLists.txt b/tests/benchmark/CMakeLists.txt index 41ed71aa64..ecacb8a921 100644 --- a/tests/benchmark/CMakeLists.txt +++ b/tests/benchmark/CMakeLists.txt @@ -19,6 +19,8 @@ add_executable(sentry_benchmark ${SENTRY_SOURCES} benchmark_init.cpp benchmark_backend.cpp + benchmark_scope.cpp + benchmark_value.cpp ) if(SENTRY_BACKEND_CRASHPAD) diff --git a/tests/benchmark/benchmark_scope.cpp b/tests/benchmark/benchmark_scope.cpp new file mode 100644 index 0000000000..48549f9020 --- /dev/null +++ b/tests/benchmark/benchmark_scope.cpp @@ -0,0 +1,98 @@ +#include + +extern "C" { +#include "sentry_core.h" +#include "sentry_options.h" +#include "sentry_scope.h" +} + +static void +noop_transport_send(sentry_envelope_t *envelope, void *) +{ + sentry_envelope_free(envelope); +} + +static sentry_options_t * +init_sdk() +{ + sentry_options_t *options = sentry_options_new(); + sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); + sentry_transport_t *transport = sentry_transport_new(noop_transport_send); + sentry_options_set_transport(options, transport); + sentry_init(options); + + sentry_set_user( + sentry_value_new_user("42", "jdoe", "j@example.com", "127.0.0.1")); + sentry_set_fingerprint("fp1", "fp2", "fp3", NULL); + + for (int i = 0; i < 50; i++) { + char key[32], val[32]; + snprintf(key, sizeof(key), "tag%d", i); + snprintf(val, sizeof(val), "value%d", i); + sentry_set_tag(key, val); + } + + sentry_value_t os = sentry_value_new_object(); + sentry_value_set_by_key(os, "name", sentry_value_new_string("macOS")); + sentry_value_set_by_key(os, "version", sentry_value_new_string("14.0")); + sentry_set_context("os", os); + + sentry_value_t device = sentry_value_new_object(); + sentry_value_set_by_key( + device, "model", sentry_value_new_string("MacBookPro")); + sentry_value_set_by_key( + device, "family", sentry_value_new_string("Macintosh")); + sentry_value_set_by_key(device, "arch", sentry_value_new_string("arm64")); + sentry_set_context("device", device); + + sentry_value_t gpu = sentry_value_new_object(); + sentry_value_set_by_key( + gpu, "name", sentry_value_new_string("Apple M1 Pro")); + sentry_value_set_by_key( + gpu, "vendor_name", sentry_value_new_string("Apple")); + sentry_set_context("gpu", gpu); + + return options; +} + +// Benchmark scope_apply_to_event — the core of every event capture. +// Clones tags, extra, contexts, user, fingerprint from scope to event. +static void +benchmark_scope_apply(benchmark::State &state) +{ + init_sdk(); + + for (auto _ : state) { + sentry_value_t event = sentry_value_new_object(); + const sentry_scope_t *scope = sentry__scope_lock(); + const sentry_options_t *opts = sentry__options_getref(); + sentry__scope_apply_to_event(scope, opts, event, SENTRY_SCOPE_NONE); + sentry_options_free((sentry_options_t *)opts); + sentry__scope_unlock(); + benchmark::DoNotOptimize(event); + sentry_value_decref(event); + } + + sentry_close(); +} + +BENCHMARK(benchmark_scope_apply); + +// Benchmark full event capture with a noop transport. +// Includes scope_apply, before_send, envelope creation, etc. +static void +benchmark_scope_capture(benchmark::State &state) +{ + init_sdk(); + + for (auto _ : state) { + sentry_value_t event + = sentry_value_new_message_event(SENTRY_LEVEL_INFO, NULL, "test"); + sentry_uuid_t id = sentry_capture_event(event); + benchmark::DoNotOptimize(id); + } + + sentry_close(); +} + +BENCHMARK(benchmark_scope_capture); diff --git a/tests/benchmark/benchmark_value.cpp b/tests/benchmark/benchmark_value.cpp new file mode 100644 index 0000000000..e75c6c370e --- /dev/null +++ b/tests/benchmark/benchmark_value.cpp @@ -0,0 +1,116 @@ +#include + +extern "C" { +#include "sentry_core.h" +} + +// Benchmark log emission with attribute cloning. +static void +noop_transport_send(sentry_envelope_t *envelope, void *) +{ + sentry_envelope_free(envelope); +} + +static void +benchmark_value_log_attributes(benchmark::State &state) +{ + sentry_options_t *options = sentry_options_new(); + sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); + sentry_options_set_enable_logs(options, true); + sentry_options_set_logs_with_attributes(options, true); + sentry_transport_t *transport = sentry_transport_new(noop_transport_send); + sentry_options_set_transport(options, transport); + sentry_init(options); + + int num_attrs = (int)state.range(0); + sentry_value_t attrs = sentry_value_new_object(); + for (int i = 0; i < num_attrs; i++) { + char key[32]; + snprintf(key, sizeof(key), "attr%d", i); + sentry_value_set_by_key(attrs, key, + sentry_value_new_attribute( + sentry_value_new_string("some_value"), NULL)); + } + + for (auto _ : state) { + sentry_value_incref(attrs); + log_return_value_t rv = sentry_log_info("test message %d", attrs, 42); + benchmark::DoNotOptimize(rv); + } + + sentry_value_decref(attrs); + sentry_close(); +} + +BENCHMARK(benchmark_value_log_attributes)->Arg(20); + +// Direct clone benchmarks — apples-to-apples comparison of sentry__value_clone. + +// Flat object (like tags): {string: string} +static void +benchmark_value_flat_object(benchmark::State &state) +{ + int n = (int)state.range(0); + sentry_value_t obj = sentry_value_new_object(); + for (int i = 0; i < n; i++) { + char key[32], val[32]; + snprintf(key, sizeof(key), "key%d", i); + snprintf(val, sizeof(val), "value%d", i); + sentry_value_set_by_key(obj, key, sentry_value_new_string(val)); + } + for (auto _ : state) { + sentry_value_t clone = sentry__value_clone(obj); + benchmark::DoNotOptimize(clone); + sentry_value_decref(clone); + } + sentry_value_decref(obj); +} + +BENCHMARK(benchmark_value_flat_object)->Arg(100); + +// Nested object (like contexts): {string: {string: string}} +static void +benchmark_value_nested_object(benchmark::State &state) +{ + int n = (int)state.range(0); + sentry_value_t obj = sentry_value_new_object(); + for (int i = 0; i < n; i++) { + char key[32]; + snprintf(key, sizeof(key), "ctx%d", i); + sentry_value_t child = sentry_value_new_object(); + sentry_value_set_by_key( + child, "name", sentry_value_new_string("some_name")); + sentry_value_set_by_key( + child, "version", sentry_value_new_string("1.0")); + sentry_value_set_by_key(obj, key, child); + } + for (auto _ : state) { + sentry_value_t clone = sentry__value_clone(obj); + benchmark::DoNotOptimize(clone); + sentry_value_decref(clone); + } + sentry_value_decref(obj); +} + +BENCHMARK(benchmark_value_nested_object)->Arg(100); + +// Flat list (like fingerprint): [string, string, ...] +static void +benchmark_value_flat_list(benchmark::State &state) +{ + int n = (int)state.range(0); + sentry_value_t list = sentry_value_new_list(); + for (int i = 0; i < n; i++) { + char val[32]; + snprintf(val, sizeof(val), "fp%d", i); + sentry_value_append(list, sentry_value_new_string(val)); + } + for (auto _ : state) { + sentry_value_t clone = sentry__value_clone(list); + benchmark::DoNotOptimize(clone); + sentry_value_decref(clone); + } + sentry_value_decref(list); +} + +BENCHMARK(benchmark_value_flat_list)->Arg(100); diff --git a/tests/conftest.py b/tests/conftest.py index 56e27eaff7..a30a0d232e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -131,7 +131,12 @@ def pytest_configure(config): def gbenchmark(): def _load(json_path, label, test_name=None): if test_name is None: - test_name = os.environ.get("PYTEST_CURRENT_TEST").split(" ")[0] + test_name = ( + os.environ.get("PYTEST_CURRENT_TEST") + .split(" ")[0] + .replace("[", "_") + .replace("]", "") + ) with open(json_path, "r") as f: data = json.load(f) @@ -168,7 +173,10 @@ def _get_benchmark(name, separator): f"Min {min(real_time):.3f}{unit}", f"Max {max(real_time):.3f}{unit}", f"Mean {statistics.mean(real_time):.3f}{unit}", - f"StdDev {statistics.stdev(real_time):.3f}{unit}", + ] + if len(real_time) > 1: + extra += [f"StdDev {statistics.stdev(real_time):.3f}{unit}"] + extra += [ f"Median {statistics.median(real_time):.3f}{unit}", f"CPU {statistics.mean(cpu_time):.3f}{unit}" if sys.platform != "win32" else "", ] @@ -183,7 +191,8 @@ def _get_benchmark(name, separator): def pytest_report_teststatus(report, config): if report.when == "call" and report.passed: - benchmark = _get_benchmark(report.nodeid, ", ") + test_name = report.nodeid.replace("[", "_").replace("]", "") + benchmark = _get_benchmark(test_name, ", ") if benchmark: return ( "passed", From 2a5834805d5f237e5156afe74c7688bbf70b6169 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 4 Jun 2026 12:40:10 +0200 Subject: [PATCH 2/3] fix: clone user and fingerprint in scope_apply_to_event --- src/sentry_scope.c | 4 +- tests/unit/test_scope.c | 82 +++++++++++++++++++++++++++++++++++++++++ tests/unit/tests.inc | 1 + 3 files changed, 85 insertions(+), 2 deletions(-) diff --git a/src/sentry_scope.c b/src/sentry_scope.c index 1c055e8ba6..ff8d0da4f9 100644 --- a/src/sentry_scope.c +++ b/src/sentry_scope.c @@ -412,10 +412,10 @@ sentry__scope_apply_to_event(const sentry_scope_t *scope, sentry_value_new_string(options->run->installation_id)); } } else if (sentry_value_get_length(scope->user) > 0) { - PLACE_VALUE("user", scope->user); + PLACE_CLONED_VALUE("user", scope->user); } } - PLACE_VALUE("fingerprint", scope->fingerprint); + PLACE_CLONED_VALUE("fingerprint", scope->fingerprint); PLACE_STRING("transaction", scope->transaction); PLACE_VALUE("sdk", scope->client_sdk); diff --git a/tests/unit/test_scope.c b/tests/unit/test_scope.c index fa74592f20..ab41256002 100644 --- a/tests/unit/test_scope.c +++ b/tests/unit/test_scope.c @@ -816,6 +816,88 @@ SENTRY_TEST(before_breadcrumb_passthrough) sentry_close(); } +static sentry_value_t +before_send_modify_scope_values( + sentry_value_t event, void *UNUSED(hint), void *UNUSED(data)) +{ + sentry_value_t contexts = sentry_value_get_by_key(event, "contexts"); + sentry_value_t gpu = sentry_value_get_by_key(contexts, "gpu"); + sentry_value_set_by_key(gpu, "name", sentry_value_new_string("modified")); + sentry_value_set_by_key( + gpu, "injected", sentry_value_new_string("injected")); + + sentry_value_t extra = sentry_value_get_by_key(event, "extra"); + sentry_value_t data_obj = sentry_value_get_by_key(extra, "data"); + sentry_value_set_by_key( + data_obj, "key", sentry_value_new_string("modified")); + sentry_value_set_by_key( + data_obj, "injected", sentry_value_new_string("injected")); + + sentry_value_t user = sentry_value_get_by_key(event, "user"); + sentry_value_set_by_key( + user, "username", sentry_value_new_string("modified")); + sentry_value_set_by_key( + user, "injected", sentry_value_new_string("injected")); + + sentry_value_t fingerprint = sentry_value_get_by_key(event, "fingerprint"); + sentry_value_append(fingerprint, sentry_value_new_string("injected")); + + return event; +} + +SENTRY_TEST(scope_clone) +{ + SENTRY_TEST_OPTIONS_NEW(options); + sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); + sentry_options_set_before_send( + options, before_send_modify_scope_values, NULL); + sentry_init(options); + + sentry_value_t gpu = sentry_value_new_object(); + sentry_value_set_by_key(gpu, "name", sentry_value_new_string("original")); + sentry_set_context("gpu", gpu); + + sentry_value_t data = sentry_value_new_object(); + sentry_value_set_by_key(data, "key", sentry_value_new_string("original")); + sentry_set_extra("data", data); + + sentry_set_user(sentry_value_new_user("1", "original", NULL, NULL)); + sentry_set_fingerprint("fp1", "fp2", NULL); + + // before_send modifies contexts, extra, user, and fingerprint on the event + sentry_capture_event( + sentry_value_new_message_event(SENTRY_LEVEL_INFO, NULL, "test")); + + // scope values must not be corrupted by before_send modifications + SENTRY_WITH_SCOPE (scope) { + sentry_value_t scope_gpu + = sentry_value_get_by_key(scope->contexts, "gpu"); + TEST_CHECK_STRING_EQUAL( + sentry_value_as_string(sentry_value_get_by_key(scope_gpu, "name")), + "original"); + TEST_CHECK(sentry_value_is_null( + sentry_value_get_by_key(scope_gpu, "injected"))); + + sentry_value_t scope_data + = sentry_value_get_by_key(scope->extra, "data"); + TEST_CHECK_STRING_EQUAL( + sentry_value_as_string(sentry_value_get_by_key(scope_data, "key")), + "original"); + TEST_CHECK(sentry_value_is_null( + sentry_value_get_by_key(scope_data, "injected"))); + + TEST_CHECK_STRING_EQUAL(sentry_value_as_string(sentry_value_get_by_key( + scope->user, "username")), + "original"); + TEST_CHECK(sentry_value_is_null( + sentry_value_get_by_key(scope->user, "injected"))); + + TEST_CHECK_INT_EQUAL(sentry_value_get_length(scope->fingerprint), 2); + } + + sentry_close(); +} + SENTRY_TEST(scope_global_attributes) { SENTRY_TEST_OPTIONS_NEW(options); diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index e7ad60ab96..90ad401680 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -275,6 +275,7 @@ XX(sampling_before_send) XX(sampling_decision) XX(sampling_transaction) XX(scope_breadcrumbs) +XX(scope_clone) XX(scope_contexts) XX(scope_extra) XX(scope_fingerprint) From ec86d1f16eaa4e3341c4a52a974119fbe96b6668 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Wed, 4 Mar 2026 11:27:37 +0100 Subject: [PATCH 3/3] ref: copy-on-write for sentry_value_t clone Add copy-on-write (COW) semantics to sentry__value_clone() for list and object values. Instead of deep-copying items/pairs on clone, the new thing_t shares the underlying list_t/obj_t data via a data-level refcount. The actual copy is deferred to mutation time (thing_detach), making clone O(1) for flat containers. Two-level refcounting: - thing_t.refcount: how many sentry_value_t handles reference the thing - list_t.refcount / obj_t.refcount: how many thing_ts share the data Container children (nested lists/objects) are still recursively cloned to maintain value semantics at each nesting level. Co-Authored-By: Claude Opus 4.6 --- src/sentry_value.c | 228 +++++++++++++++++++++++++++++++++------ tests/unit/test_value.c | 232 ++++++++++++++++++++++++++++++++++++++++ tests/unit/tests.inc | 9 ++ 3 files changed, 439 insertions(+), 30 deletions(-) diff --git a/src/sentry_value.c b/src/sentry_value.c index 6a1ea6d06a..fb076b13d1 100644 --- a/src/sentry_value.c +++ b/src/sentry_value.c @@ -93,6 +93,7 @@ typedef struct { sentry_value_t *items; size_t len; size_t allocated; + long refcount; } list_t; typedef struct { @@ -104,6 +105,7 @@ typedef struct { obj_pair_t *pairs; size_t len; size_t allocated; + long refcount; } obj_t; static const char * @@ -161,33 +163,143 @@ thing_get_type(const thing_t *thing) } static void -thing_free(thing_t *thing) +list_free(list_t *list) +{ + if (sentry__atomic_fetch_and_add(&list->refcount, -1) != 1) { + return; + } + for (size_t i = 0; i < list->len; i++) { + sentry_value_decref(list->items[i]); + } + sentry_free(list->items); + sentry_free(list); +} + +static void +obj_free(obj_t *obj) +{ + if (sentry__atomic_fetch_and_add(&obj->refcount, -1) != 1) { + return; + } + for (size_t i = 0; i < obj->len; i++) { + sentry_free(obj->pairs[i].k); + sentry_value_decref(obj->pairs[i].v); + } + sentry_free(obj->pairs); + sentry_free(obj); +} + +static bool +thing_detach(thing_t *thing) { switch (thing_get_type(thing)) { case THING_TYPE_LIST: { - list_t *list = thing->payload._ptr; - for (size_t i = 0; i < list->len; i++) { - sentry_value_decref(list->items[i]); + list_t *old = thing->payload._ptr; + if (sentry__atomic_fetch(&old->refcount) <= 1) { + return true; } - sentry_free(list->items); - sentry_free(list); - break; + list_t *new_list = SENTRY_MAKE(list_t); + if (!new_list) { + return false; + } + new_list->len = old->len; + new_list->allocated = old->len; + new_list->refcount = 1; + if (old->len) { + new_list->items = sentry_malloc(sizeof(sentry_value_t) * old->len); + if (!new_list->items) { + sentry_free(new_list); + return false; + } + memcpy( + new_list->items, old->items, sizeof(sentry_value_t) * old->len); + for (size_t i = 0; i < old->len; i++) { + sentry_value_incref(new_list->items[i]); + } + } else { + new_list->items = NULL; + } + list_free(old); + thing->payload._ptr = new_list; + return true; } case THING_TYPE_OBJECT: { - obj_t *obj = thing->payload._ptr; - for (size_t i = 0; i < obj->len; i++) { - sentry_free(obj->pairs[i].k); - sentry_value_decref(obj->pairs[i].v); + obj_t *old = thing->payload._ptr; + if (sentry__atomic_fetch(&old->refcount) <= 1) { + return true; + } + obj_t *new_obj = SENTRY_MAKE(obj_t); + if (!new_obj) { + return false; + } + new_obj->len = old->len; + new_obj->allocated = old->len; + new_obj->refcount = 1; + if (old->len) { + new_obj->pairs = sentry_malloc(sizeof(obj_pair_t) * old->len); + if (!new_obj->pairs) { + sentry_free(new_obj); + return false; + } + for (size_t i = 0; i < old->len; i++) { + new_obj->pairs[i].k = sentry__string_clone(old->pairs[i].k); + new_obj->pairs[i].v = old->pairs[i].v; + sentry_value_incref(new_obj->pairs[i].v); + } + } else { + new_obj->pairs = NULL; } - sentry_free(obj->pairs); - sentry_free(obj); + obj_free(old); + thing->payload._ptr = new_obj; + return true; + } + default: + return true; + } +} + +static sentry_value_t +thing_get_child(const thing_t *thing, size_t i) +{ + switch (thing_get_type(thing)) { + case THING_TYPE_LIST: + return ((const list_t *)thing->payload._ptr)->items[i]; + case THING_TYPE_OBJECT: + return ((const obj_t *)thing->payload._ptr)->pairs[i].v; + default: + return sentry_value_new_null(); + } +} + +static void +thing_set_child(thing_t *thing, size_t i, sentry_value_t value) +{ + switch (thing_get_type(thing)) { + case THING_TYPE_LIST: + ((list_t *)thing->payload._ptr)->items[i] = value; + break; + case THING_TYPE_OBJECT: + ((obj_t *)thing->payload._ptr)->pairs[i].v = value; + break; + default: break; } - case THING_TYPE_STRING: { +} + +static void +thing_free(thing_t *thing) +{ + switch (thing_get_type(thing)) { + case THING_TYPE_LIST: + list_free(thing->payload._ptr); + break; + case THING_TYPE_OBJECT: + obj_free(thing->payload._ptr); + break; + case THING_TYPE_STRING: sentry_free(thing->payload._ptr); break; } - } sentry_free(thing); } @@ -395,6 +507,8 @@ sentry_value_new_list(void) { list_t *l = SENTRY_MAKE(list_t); if (l) { + memset(l, 0, sizeof(list_t)); + l->refcount = 1; sentry_value_t rv = new_thing_value(l, THING_TYPE_LIST); if (sentry_value_is_null(rv)) { sentry_free(l); @@ -410,6 +524,8 @@ sentry__value_new_list_with_size(size_t size) { list_t *l = SENTRY_MAKE(list_t); if (l) { + memset(l, 0, sizeof(list_t)); + l->refcount = 1; l->allocated = size; if (size) { l->items = sentry_malloc(sizeof(sentry_value_t) * size); @@ -434,6 +550,8 @@ sentry_value_new_object(void) { obj_t *o = SENTRY_MAKE(obj_t); if (o) { + memset(o, 0, sizeof(obj_t)); + o->refcount = 1; sentry_value_t rv = new_thing_value(o, THING_TYPE_OBJECT); if (sentry_value_is_null(rv)) { sentry_free(o); @@ -449,6 +567,8 @@ sentry__value_new_object_with_size(size_t size) { obj_t *o = SENTRY_MAKE(obj_t); if (o) { + memset(o, 0, sizeof(obj_t)); + o->refcount = 1; o->allocated = size; if (size) { o->pairs = sentry_malloc(sizeof(obj_pair_t) * size); @@ -639,7 +759,8 @@ sentry_value_set_by_key_n( } sentry_slice_t k_slice = { k, k_len }; thing_t *thing = value_as_unfrozen_thing(value); - if (!thing || thing_get_type(thing) != THING_TYPE_OBJECT) { + if (!thing || thing_get_type(thing) != THING_TYPE_OBJECT + || !thing_detach(thing)) { goto fail; } obj_t *o = thing->payload._ptr; @@ -690,7 +811,8 @@ sentry_value_remove_by_key_n(sentry_value_t value, const char *k, size_t k_len) } sentry_slice_t k_slice = { k, k_len }; thing_t *thing = value_as_unfrozen_thing(value); - if (!thing || thing_get_type(thing) != THING_TYPE_OBJECT) { + if (!thing || thing_get_type(thing) != THING_TYPE_OBJECT + || !thing_detach(thing)) { return 1; } obj_t *o = thing->payload._ptr; @@ -722,7 +844,8 @@ int sentry_value_append(sentry_value_t value, sentry_value_t v) { thing_t *thing = value_as_unfrozen_thing(value); - if (!thing || thing_get_type(thing) != THING_TYPE_LIST) { + if (!thing || thing_get_type(thing) != THING_TYPE_LIST + || !thing_detach(thing)) { goto fail; } @@ -791,6 +914,39 @@ sentry__value_stringify(sentry_value_t value) #undef STRINGIFY_NUMERIC } +static bool +value_is_container(sentry_value_t value) +{ + const thing_t *thing = value_as_thing(value); + if (!thing) { + return false; + } + int type = thing_get_type(thing); + return type == THING_TYPE_LIST || type == THING_TYPE_OBJECT; +} + +static bool +thing_clone_children(thing_t *thing, size_t len) +{ + bool detached = false; + for (size_t i = 0; i < len; i++) { + sentry_value_t child = thing_get_child(thing, i); + if (!value_is_container(child)) { + continue; + } + if (!detached) { + if (!thing_detach(thing)) { + return false; + } + detached = true; + } + sentry_value_t cloned = sentry__value_clone(child); + sentry_value_decref(child); + thing_set_child(thing, i, cloned); + } + return true; +} + sentry_value_t sentry__value_clone(sentry_value_t value) { @@ -800,20 +956,30 @@ sentry__value_clone(sentry_value_t value) } switch (thing_get_type(thing)) { case THING_TYPE_LIST: { - const list_t *list = thing->payload._ptr; - sentry_value_t rv = sentry__value_new_list_with_size(list->len); - for (size_t i = 0; i < list->len; i++) { - sentry_value_incref(list->items[i]); - sentry_value_append(rv, list->items[i]); + list_t *list = thing->payload._ptr; + sentry__atomic_fetch_and_add(&list->refcount, 1); + sentry_value_t rv = new_thing_value(list, THING_TYPE_LIST); + if (sentry_value_is_null(rv)) { + list_free(list); + return rv; + } + if (!thing_clone_children(value_as_thing(rv), list->len)) { + sentry_value_decref(rv); + return sentry_value_new_null(); } return rv; } case THING_TYPE_OBJECT: { - const obj_t *obj = thing->payload._ptr; - sentry_value_t rv = sentry__value_new_object_with_size(obj->len); - for (size_t i = 0; i < obj->len; i++) { - sentry_value_incref(obj->pairs[i].v); - sentry_value_set_by_key(rv, obj->pairs[i].k, obj->pairs[i].v); + obj_t *obj = thing->payload._ptr; + sentry__atomic_fetch_and_add(&obj->refcount, 1); + sentry_value_t rv = new_thing_value(obj, THING_TYPE_OBJECT); + if (sentry_value_is_null(rv)) { + obj_free(obj); + return rv; + } + if (!thing_clone_children(value_as_thing(rv), obj->len)) { + sentry_value_decref(rv); + return sentry_value_new_null(); } return rv; } @@ -832,7 +998,8 @@ int sentry_value_set_by_index(sentry_value_t value, size_t index, sentry_value_t v) { thing_t *thing = value_as_unfrozen_thing(value); - if (!thing || thing_get_type(thing) != THING_TYPE_LIST) { + if (!thing || thing_get_type(thing) != THING_TYPE_LIST + || !thing_detach(thing)) { goto fail; } @@ -862,7 +1029,8 @@ int sentry_value_remove_by_index(sentry_value_t value, size_t index) { thing_t *thing = value_as_unfrozen_thing(value); - if (!thing || thing_get_type(thing) != THING_TYPE_LIST) { + if (!thing || thing_get_type(thing) != THING_TYPE_LIST + || !thing_detach(thing)) { return 1; } diff --git a/tests/unit/test_value.c b/tests/unit/test_value.c index ba0ef57c23..c966b6d19f 100644 --- a/tests/unit/test_value.c +++ b/tests/unit/test_value.c @@ -436,6 +436,238 @@ SENTRY_TEST(value_object_merge_nested) sentry_value_decref(dst); } +SENTRY_TEST(value_clone_list) +{ + sentry_value_t original = sentry_value_new_list(); + sentry_value_append(original, sentry_value_new_int32(1)); + sentry_value_append(original, sentry_value_new_int32(2)); + sentry_value_append(original, sentry_value_new_int32(3)); + + sentry_value_t clone = sentry__value_clone(original); + TEST_CHECK(sentry_value_refcount(clone) == 1); + TEST_CHECK_JSON_VALUE(clone, "[1,2,3]"); + + // mutating the clone triggers COW — original is unaffected + sentry_value_append(clone, sentry_value_new_int32(4)); + TEST_CHECK_JSON_VALUE(clone, "[1,2,3,4]"); + TEST_CHECK_JSON_VALUE(original, "[1,2,3]"); + + sentry_value_set_by_index(clone, 0, sentry_value_new_int32(10)); + TEST_CHECK_JSON_VALUE(clone, "[10,2,3,4]"); + TEST_CHECK_JSON_VALUE(original, "[1,2,3]"); + + sentry_value_remove_by_index(clone, 1); + TEST_CHECK_JSON_VALUE(clone, "[10,3,4]"); + TEST_CHECK_JSON_VALUE(original, "[1,2,3]"); + + sentry_value_decref(clone); + // original still valid after clone is freed + TEST_CHECK_JSON_VALUE(original, "[1,2,3]"); + sentry_value_decref(original); +} + +SENTRY_TEST(value_clone_object) +{ + sentry_value_t original = sentry_value_new_object(); + sentry_value_set_by_key(original, "a", sentry_value_new_int32(1)); + sentry_value_set_by_key(original, "b", sentry_value_new_int32(2)); + + sentry_value_t clone = sentry__value_clone(original); + TEST_CHECK(sentry_value_refcount(clone) == 1); + TEST_CHECK_JSON_VALUE(clone, "{\"a\":1,\"b\":2}"); + + // mutating the clone triggers COW — original is unaffected + sentry_value_set_by_key(clone, "a", sentry_value_new_int32(10)); + TEST_CHECK_INT_EQUAL( + sentry_value_as_int32(sentry_value_get_by_key(clone, "a")), 10); + TEST_CHECK_INT_EQUAL( + sentry_value_as_int32(sentry_value_get_by_key(original, "a")), 1); + + sentry_value_set_by_key(clone, "c", sentry_value_new_int32(3)); + TEST_CHECK(sentry_value_get_length(clone) == 3); + TEST_CHECK(sentry_value_get_length(original) == 2); + + sentry_value_remove_by_key(clone, "b"); + TEST_CHECK(sentry_value_get_length(clone) == 2); + TEST_CHECK(sentry_value_get_length(original) == 2); + + sentry_value_decref(clone); + TEST_CHECK_JSON_VALUE(original, "{\"a\":1,\"b\":2}"); + sentry_value_decref(original); +} + +SENTRY_TEST(value_clone_frozen) +{ + sentry_value_t original = sentry_value_new_object(); + sentry_value_set_by_key(original, "a", sentry_value_new_int32(1)); + sentry_value_freeze(original); + TEST_CHECK(sentry_value_is_frozen(original)); + + sentry_value_t clone = sentry__value_clone(original); + TEST_CHECK(!sentry_value_is_frozen(clone)); + + // clone is mutable even though original is frozen + sentry_value_set_by_key(clone, "b", sentry_value_new_int32(2)); + TEST_CHECK_JSON_VALUE(clone, "{\"a\":1,\"b\":2}"); + TEST_CHECK_JSON_VALUE(original, "{\"a\":1}"); + + sentry_value_decref(clone); + sentry_value_decref(original); +} + +SENTRY_TEST(value_clone_multiple) +{ + sentry_value_t original = sentry_value_new_list(); + sentry_value_append(original, sentry_value_new_int32(1)); + + sentry_value_t clone1 = sentry__value_clone(original); + sentry_value_t clone2 = sentry__value_clone(original); + + // each clone can be mutated independently + sentry_value_append(clone1, sentry_value_new_int32(2)); + sentry_value_append(clone2, sentry_value_new_int32(3)); + + TEST_CHECK_JSON_VALUE(original, "[1]"); + TEST_CHECK_JSON_VALUE(clone1, "[1,2]"); + TEST_CHECK_JSON_VALUE(clone2, "[1,3]"); + + sentry_value_decref(original); + sentry_value_decref(clone1); + sentry_value_decref(clone2); +} + +SENTRY_TEST(value_clone_of_clone) +{ + sentry_value_t original = sentry_value_new_object(); + sentry_value_set_by_key(original, "x", sentry_value_new_int32(1)); + + sentry_value_t clone1 = sentry__value_clone(original); + sentry_value_t clone2 = sentry__value_clone(clone1); + + sentry_value_set_by_key(clone2, "y", sentry_value_new_int32(2)); + TEST_CHECK_JSON_VALUE(original, "{\"x\":1}"); + TEST_CHECK_JSON_VALUE(clone1, "{\"x\":1}"); + TEST_CHECK_JSON_VALUE(clone2, "{\"x\":1,\"y\":2}"); + + sentry_value_decref(original); + sentry_value_decref(clone1); + sentry_value_decref(clone2); +} + +SENTRY_TEST(value_clone_merge) +{ + sentry_value_t original = sentry_value_new_object(); + sentry_value_set_by_key(original, "a", sentry_value_new_int32(1)); + + sentry_value_t clone = sentry__value_clone(original); + + sentry_value_t src = sentry_value_new_object(); + sentry_value_set_by_key(src, "b", sentry_value_new_int32(2)); + + int rv = sentry__value_merge_objects(clone, src); + TEST_CHECK_INT_EQUAL(rv, 0); + sentry_value_decref(src); + + TEST_CHECK_JSON_VALUE(clone, "{\"a\":1,\"b\":2}"); + TEST_CHECK_JSON_VALUE(original, "{\"a\":1}"); + + sentry_value_decref(clone); + sentry_value_decref(original); +} + +SENTRY_TEST(value_clone_free_original_first) +{ + sentry_value_t original = sentry_value_new_list(); + sentry_value_append(original, sentry_value_new_string("hello")); + sentry_value_append(original, sentry_value_new_int32(42)); + + sentry_value_t clone = sentry__value_clone(original); + // free original while clone still shares data + sentry_value_decref(original); + + TEST_CHECK_JSON_VALUE(clone, "[\"hello\",42]"); + sentry_value_append(clone, sentry_value_new_int32(3)); + TEST_CHECK_JSON_VALUE(clone, "[\"hello\",42,3]"); + sentry_value_decref(clone); +} + +SENTRY_TEST(value_clone_nested_modify_leaf) +{ + // {ctx: {name: "os", ver: "14"}, tag: "v1"} + sentry_value_t original = sentry_value_new_object(); + sentry_value_t child = sentry_value_new_object(); + sentry_value_set_by_key(child, "name", sentry_value_new_string("os")); + sentry_value_set_by_key(child, "ver", sentry_value_new_string("14")); + sentry_value_set_by_key(original, "ctx", child); + sentry_value_set_by_key(original, "tag", sentry_value_new_string("v1")); + + sentry_value_t clone = sentry__value_clone(original); + + // mutate a leaf key on the clone — original unaffected + sentry_value_set_by_key(clone, "tag", sentry_value_new_string("v2")); + TEST_CHECK_STRING_EQUAL( + sentry_value_as_string(sentry_value_get_by_key(original, "tag")), "v1"); + TEST_CHECK_STRING_EQUAL( + sentry_value_as_string(sentry_value_get_by_key(clone, "tag")), "v2"); + + // nested child objects are independent + sentry_value_t clone_ctx = sentry_value_get_by_key(clone, "ctx"); + sentry_value_set_by_key(clone_ctx, "name", sentry_value_new_string("win")); + sentry_value_set_by_key(clone_ctx, "arch", sentry_value_new_string("x64")); + + sentry_value_t orig_ctx = sentry_value_get_by_key(original, "ctx"); + TEST_CHECK_STRING_EQUAL( + sentry_value_as_string(sentry_value_get_by_key(orig_ctx, "name")), + "os"); + TEST_CHECK(sentry_value_is_null(sentry_value_get_by_key(orig_ctx, "arch"))); + TEST_CHECK(sentry_value_get_length(orig_ctx) == 2); + + TEST_CHECK_STRING_EQUAL( + sentry_value_as_string(sentry_value_get_by_key(clone_ctx, "name")), + "win"); + TEST_CHECK_STRING_EQUAL( + sentry_value_as_string(sentry_value_get_by_key(clone_ctx, "arch")), + "x64"); + TEST_CHECK(sentry_value_get_length(clone_ctx) == 3); + + sentry_value_decref(clone); + sentry_value_decref(original); +} + +SENTRY_TEST(value_clone_nested_modify_middle) +{ + // [[1, 2], [3, 4]] + sentry_value_t original = sentry_value_new_list(); + sentry_value_t inner0 = sentry_value_new_list(); + sentry_value_append(inner0, sentry_value_new_int32(1)); + sentry_value_append(inner0, sentry_value_new_int32(2)); + sentry_value_t inner1 = sentry_value_new_list(); + sentry_value_append(inner1, sentry_value_new_int32(3)); + sentry_value_append(inner1, sentry_value_new_int32(4)); + sentry_value_append(original, inner0); + sentry_value_append(original, inner1); + + sentry_value_t clone = sentry__value_clone(original); + + // replace a whole nested list in the clone (middle-level mutation) + sentry_value_t replacement = sentry_value_new_list(); + sentry_value_append(replacement, sentry_value_new_int32(99)); + sentry_value_set_by_index(clone, 0, replacement); + + TEST_CHECK_JSON_VALUE(original, "[[1,2],[3,4]]"); + TEST_CHECK_JSON_VALUE(clone, "[[99],[3,4]]"); + + // mutate a leaf inside the other nested list + sentry_value_t clone_inner1 = sentry_value_get_by_index(clone, 1); + sentry_value_append(clone_inner1, sentry_value_new_int32(5)); + + TEST_CHECK_JSON_VALUE(original, "[[1,2],[3,4]]"); + TEST_CHECK_JSON_VALUE(clone, "[[99],[3,4,5]]"); + + sentry_value_decref(clone); + sentry_value_decref(original); +} + SENTRY_TEST(value_user) { const char *id = "42"; diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 90ad401680..94007eb747 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -357,6 +357,15 @@ XX(uuid_api) XX(uuid_v4) XX(value_attribute) XX(value_bool) +XX(value_clone_free_original_first) +XX(value_clone_frozen) +XX(value_clone_list) +XX(value_clone_merge) +XX(value_clone_multiple) +XX(value_clone_nested_modify_leaf) +XX(value_clone_nested_modify_middle) +XX(value_clone_object) +XX(value_clone_of_clone) XX(value_double) XX(value_foreach_key_value) XX(value_freezing)