From 00c23b6c396a40b2d0b35683afbb146aaa1f1faf Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 17 Sep 2026 15:55:31 +0200 Subject: [PATCH 1/3] perf: add support for batched scope updates Keep the scope write lock across a batch while allowing the owning thread to use existing scope accessors without relocking. Defer backend scope flushes until the outermost batch completes. --- CHANGELOG.md | 1 + include/sentry.h | 18 ++++ src/sentry_scope.c | 176 ++++++++++++++++++++++++++++++++++------ tests/unit/test_scope.c | 73 +++++++++++++++++ tests/unit/tests.inc | 2 + 5 files changed, 244 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 001371ca3..cef11e4a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ - Add `sentry_attachment_from_file/bytes` (and their wide-string variants) for creating attachment values that can be fully configured before they are added. ([#2079](https://github.com/getsentry/sentry-native/pull/2079)) - Add `sentry_add_attachment`, `sentry_scope_add_attachment`, and `sentry_hint_add_attachment` for adding configured attachments to the global scope, a specific scope, or a hint. These functions consume and freeze the attachment value. ([#2079](https://github.com/getsentry/sentry-native/pull/2079), [#1974](https://github.com/getsentry/sentry-native/pull/1974)) - Add `sentry_start_new_trace()` as a clearer name for starting a new trace. ([#2095](https://github.com/getsentry/sentry-native/pull/2095)) +- Add `sentry_scope_begin_update` and `sentry_scope_end_update` for applying multiple scope changes with a single scope flush, improving bulk-update performance. ([#2107](https://github.com/getsentry/sentry-native/pull/2107)) **Fixes**: diff --git a/include/sentry.h b/include/sentry.h index 2516a8322..a1fb9ff96 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -2544,6 +2544,24 @@ SENTRY_API sentry_scope_t *sentry_local_scope_new(void); */ SENTRY_API sentry_scope_t *sentry_scope_new(void); +/** + * Begins a batch update of `scope`. + * + * The scope remains write-locked until the matching + * `sentry_scope_end_update` call on the same thread. Scope functions may be + * called normally during the update, including functions that read the scope. + * Updates may be nested for the same scope. + */ +SENTRY_API void sentry_scope_begin_update(sentry_scope_t *scope); + +/** + * Ends a batch update begun by `sentry_scope_begin_update`. + * + * The outermost call releases the scope write lock and flushes any pending + * global scope changes to the backend. + */ +SENTRY_API void sentry_scope_end_update(sentry_scope_t *scope); + /** * Frees a scope created via `sentry_scope_new`, `sentry_scope_clone`, or * `sentry_local_scope_new`. diff --git a/src/sentry_scope.c b/src/sentry_scope.c index 27c2c8dd6..7f62db094 100644 --- a/src/sentry_scope.c +++ b/src/sentry_scope.c @@ -30,6 +30,8 @@ struct sentry_scope_data_s { sentry_rwlock_t rwlock; + uint64_t update_owner; + size_t update_depth; sentry_value_t release; sentry_value_t environment; @@ -68,8 +70,12 @@ static bool g_scope_idle_initialized = false; static sentry_cond_t g_scope_idle; #ifdef _MSC_VER static __declspec(thread) size_t g_scope_depth; +static __declspec(thread) size_t g_scope_update_depth; +static __declspec(thread) char g_scope_update_token; #else static __thread size_t g_scope_depth; +static __thread size_t g_scope_update_depth; +static __thread char g_scope_update_token; #endif #ifdef SENTRY__MUTEX_INIT_DYN SENTRY__MUTEX_INIT_DYN(g_lock) @@ -77,23 +83,81 @@ SENTRY__MUTEX_INIT_DYN(g_lock) static sentry_mutex_t g_lock = SENTRY__MUTEX_INIT; #endif +static uint64_t +scope_update_token(void) +{ + return (uint64_t)(uintptr_t)&g_scope_update_token; +} + +static bool +scope_data_is_updating(const sentry_scope_data_t *data) +{ +#ifndef SENTRY_PLATFORM_WINDOWS + // avoid dynamic TLS access during signal handling + if (!sentry__block_for_signal_handler()) { + return false; + } +#endif + if (!g_scope_update_depth) { + return false; + } + return sentry__atomic_fetch_u64((uint64_t *)&data->update_owner) + == scope_update_token(); +} + +static bool +scope_data_read_lock(const sentry_scope_data_t *data) +{ + if (scope_data_is_updating(data)) { + return false; + } + sentry__rwlock_read_lock((sentry_rwlock_t *)&data->rwlock); + return true; +} + +static void +scope_data_read_unlock(const sentry_scope_data_t *data, bool locked) +{ + if (locked) { + sentry__rwlock_read_unlock((sentry_rwlock_t *)&data->rwlock); + } +} + +static bool +scope_data_write_lock(sentry_scope_data_t *data) +{ + if (scope_data_is_updating(data)) { + return false; + } + sentry__rwlock_write_lock(&data->rwlock); + return true; +} + +static void +scope_data_write_unlock(sentry_scope_data_t *data, bool locked) +{ + if (locked) { + sentry__rwlock_write_unlock(&data->rwlock); + } +} + #define SENTRY_SCOPE_READ_LOCK(Data) \ for (const sentry_scope_data_t *_locked_data = (Data); _locked_data; \ - sentry__rwlock_read_unlock((sentry_rwlock_t *)&_locked_data->rwlock), \ - _locked_data = NULL) \ - for (bool _locked_once \ - = (sentry__rwlock_read_lock( \ - (sentry_rwlock_t *)&_locked_data->rwlock), \ - true); \ - _locked_once; _locked_once = false) + _locked_data = NULL) \ + for (bool _lock_acquired = scope_data_read_lock(_locked_data), \ + _locked_once = true; \ + _locked_once; \ + scope_data_read_unlock(_locked_data, _lock_acquired), \ + _locked_once = false) #define SENTRY_SCOPE_WRITE_LOCK(Data) \ for (sentry_scope_data_t *_locked_data = (Data); _locked_data; \ - sentry__rwlock_write_unlock(&_locked_data->rwlock), \ - _locked_data = NULL) \ - for (bool _locked_once \ - = (sentry__rwlock_write_lock(&_locked_data->rwlock), true); \ - _locked_once; _locked_once = false) + _locked_data = NULL) \ + for (bool _lock_acquired = scope_data_write_lock(_locked_data), \ + _locked_once = true; \ + _locked_once; \ + scope_data_write_unlock(_locked_data, _lock_acquired), \ + _locked_once = false) static size_t begin_scope_notify(sentry_scope_t *scope); static void end_scope_notify(sentry_scope_t *scope); @@ -533,33 +597,46 @@ sentry__scope_finish(sentry_scope_t *scope) sentry__scope_decref(scope); } -void -sentry__scope_finish_mut(sentry_scope_t *scope, bool flush) +static bool +scope_should_flush(sentry_scope_t *scope, bool flush) { - if (!scope) { - return; - } - sentry__mutex_lock(&scope->observers_lock); - if (scope->is_notifying > 0) { - // defer the flush requested by a reentrant scope change + if (scope->is_notifying > 0 || scope_data_is_updating(scope->data)) { + // defer the flush requested by a reentrant or batched scope change scope->pending_flush = flush || scope->pending_flush; flush = false; } else { - // consume any flush requested by a reentrant scope change + // consume any flush requested by a reentrant or batched scope change flush = flush || scope->pending_flush; scope->pending_flush = false; } sentry__mutex_unlock(&scope->observers_lock); + return flush; +} + +static void +flush_scope(void) +{ + SENTRY_WITH_OPTIONS (options) { + if (options->backend && options->backend->flush_scope_func) { + options->backend->flush_scope_func(options->backend, options); + } + } +} + +void +sentry__scope_finish_mut(sentry_scope_t *scope, bool flush) +{ + if (!scope) { + return; + } + + flush = scope_should_flush(scope, flush); sentry__scope_finish(scope); if (flush) { - SENTRY_WITH_OPTIONS (options) { - if (options->backend && options->backend->flush_scope_func) { - options->backend->flush_scope_func(options->backend, options); - } - } + flush_scope(); } } @@ -694,6 +771,53 @@ sentry_scope_new(void) return scope; } +void +sentry_scope_begin_update(sentry_scope_t *scope) +{ + if (!scope) { + return; + } + + sentry_scope_data_t *data = scope->data; + if (scope_data_is_updating(data)) { + data->update_depth++; + return; + } + + lock_scope_notify(scope); + sentry__rwlock_write_lock(&data->rwlock); + data->update_depth = 1; + sentry__atomic_store_u64(&data->update_owner, scope_update_token()); + g_scope_update_depth++; +} + +void +sentry_scope_end_update(sentry_scope_t *scope) +{ + if (!scope) { + return; + } + + sentry_scope_data_t *data = scope->data; + if (!scope_data_is_updating(data)) { + assert(false); + return; + } + if (--data->update_depth > 0) { + return; + } + + sentry__atomic_store_u64(&data->update_owner, 0); + g_scope_update_depth--; + sentry__rwlock_write_unlock(&data->rwlock); + + bool flush = scope_should_flush(scope, scope == &g_scope); + unlock_scope_notify(scope); + if (flush && scope == &g_scope) { + flush_scope(); + } +} + void sentry_scope_free(sentry_scope_t *scope) { diff --git a/tests/unit/test_scope.c b/tests/unit/test_scope.c index 118212bc6..70cde6d70 100644 --- a/tests/unit/test_scope.c +++ b/tests/unit/test_scope.c @@ -35,6 +35,68 @@ scope_value_get_length(scope_value_getter_t get, const sentry_scope_t *scope) return length; } +SENTRY_TEST(scope_update_nested) +{ + sentry_scope_t *scope = sentry_scope_new(); + TEST_ASSERT(!!scope); + + sentry_scope_begin_update(scope); + sentry_scope_set_tag(scope, "first", "value"); + + sentry_scope_begin_update(scope); + sentry_scope_set_tag(scope, "second", "value"); + TEST_CHECK(scope_value_get_length(sentry__scope_load_tags, scope) == 2); + sentry_scope_end_update(scope); + + sentry_scope_set_environment(scope, "production"); + sentry_value_t environment = sentry__scope_ref_environment(scope); + TEST_CHECK_STRING_EQUAL(sentry_value_as_string(environment), "production"); + sentry_value_decref(environment); + sentry_scope_end_update(scope); + + TEST_CHECK(scope_value_get_length(sentry__scope_load_tags, scope) == 2); + sentry_scope_free(scope); +} + +typedef struct { + sentry_scope_t *scope; + sentry_waitable_flag_t started; + sentry_waitable_flag_t finished; +} scope_update_thread_t; + +SENTRY_THREAD_FN +read_scope_during_update(void *data) +{ + scope_update_thread_t *state = data; + sentry__waitable_flag_set(&state->started); + sentry_value_decref(sentry__scope_load_tags(state->scope)); + sentry__waitable_flag_set(&state->finished); + return 0; +} + +SENTRY_TEST(scope_update_exclusive) +{ + sentry_scope_t *scope = sentry_scope_new(); + TEST_ASSERT(!!scope); + + scope_update_thread_t state = { .scope = scope }; + sentry__waitable_flag_init(&state.started); + sentry__waitable_flag_init(&state.finished); + + sentry_scope_begin_update(scope); + sentry_threadid_t thread; + sentry__thread_init(&thread); + TEST_ASSERT( + !sentry__thread_spawn(&thread, read_scope_during_update, &state)); + TEST_ASSERT(sentry__waitable_flag_wait(&state.started, 1000)); + TEST_CHECK(!sentry__waitable_flag_wait(&state.finished, 10)); + + sentry_scope_end_update(scope); + TEST_CHECK(sentry__waitable_flag_wait(&state.finished, 1000)); + sentry__thread_join(thread); + sentry_scope_free(scope); +} + SENTRY_TEST(scope_contexts) { SENTRY_TEST_OPTIONS_NEW(options); @@ -2060,6 +2122,17 @@ SENTRY_TEST(scope_observer_deferred_flush) TEST_CHECK_INT_EQUAL(observer_data.nested_flush_count, 0); TEST_CHECK_INT_EQUAL(observer_data.total_flush_count, 1); + observer_data.total_flush_count = 0; + observer_data.nested_flush_count = 0; + sentry_scope_t *scope = sentry__scope_getref(); + sentry_scope_begin_update(scope); + sentry_scope_set_tag(scope, "batched", "value"); + TEST_CHECK_INT_EQUAL(observer_data.nested_flush_count, 0); + TEST_CHECK_INT_EQUAL(observer_data.total_flush_count, 0); + sentry_scope_end_update(scope); + TEST_CHECK_INT_EQUAL(observer_data.total_flush_count, 1); + sentry__scope_finish(scope); + sentry_close(); } diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 623e5bc97..55cabd710 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -423,6 +423,8 @@ XX(scope_tags) XX(scope_tags_flush) XX(scope_transaction) XX(scope_update_context) +XX(scope_update_exclusive) +XX(scope_update_nested) XX(scope_user) XX(scope_user_id) XX(scoped_txn) From b8971c9a3fc8ee6beba583ff9cb3a349c28b73a7 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Fri, 18 Sep 2026 16:06:49 +0200 Subject: [PATCH 2/3] begin/end_read + begin/end_write --- CHANGELOG.md | 2 +- include/sentry.h | 47 ++++++++++-- src/sentry_scope.c | 137 ++++++++++++++++++++++----------- tests/unit/test_scope.c | 166 ++++++++++++++++++++++++++++++++++++---- tests/unit/tests.inc | 7 +- 5 files changed, 288 insertions(+), 71 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cef11e4a8..62ca36e73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ - Add `sentry_attachment_from_file/bytes` (and their wide-string variants) for creating attachment values that can be fully configured before they are added. ([#2079](https://github.com/getsentry/sentry-native/pull/2079)) - Add `sentry_add_attachment`, `sentry_scope_add_attachment`, and `sentry_hint_add_attachment` for adding configured attachments to the global scope, a specific scope, or a hint. These functions consume and freeze the attachment value. ([#2079](https://github.com/getsentry/sentry-native/pull/2079), [#1974](https://github.com/getsentry/sentry-native/pull/1974)) - Add `sentry_start_new_trace()` as a clearer name for starting a new trace. ([#2095](https://github.com/getsentry/sentry-native/pull/2095)) -- Add `sentry_scope_begin_update` and `sentry_scope_end_update` for applying multiple scope changes with a single scope flush, improving bulk-update performance. ([#2107](https://github.com/getsentry/sentry-native/pull/2107)) +- Add `sentry_scope_begin_read`/`sentry_scope_end_read` and `sentry_scope_begin_write`/`sentry_scope_end_write` for consistent reads of multiple scope properties and applying multiple scope changes with a single scope flush. ([#2107](https://github.com/getsentry/sentry-native/pull/2107)) **Fixes**: diff --git a/include/sentry.h b/include/sentry.h index a1fb9ff96..4800ecf9e 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -2545,22 +2545,53 @@ SENTRY_API sentry_scope_t *sentry_local_scope_new(void); SENTRY_API sentry_scope_t *sentry_scope_new(void); /** - * Begins a batch update of `scope`. + * Begins a batch read of `scope`. + * + * The scope remains read-locked until the matching `sentry_scope_end_read` call + * on the same thread, so multiple properties can be read consistently. Other + * threads may read the scope concurrently. Reads may be nested for the same + * scope, including inside a write batch. Mutating the scope inside an outermost + * read batch is not allowed. + * + * Returns 0 on success or 1 if `scope` is NULL or allocation fails. Each + * successful call must be matched by `sentry_scope_end_read` before freeing the + * scope. + */ +SENTRY_API int sentry_scope_begin_read(const sentry_scope_t *scope); + +/** + * Ends a read batch begun by `sentry_scope_begin_read`. + * + * The outermost call releases the scope read lock. Reads nested inside a write + * batch leave the scope write-locked until the outermost + * `sentry_scope_end_write`. This must be called on the same thread as the + * matching begin call. + */ +SENTRY_API void sentry_scope_end_read(const sentry_scope_t *scope); + +/** + * Begins a batch write of `scope`. * * The scope remains write-locked until the matching - * `sentry_scope_end_update` call on the same thread. Scope functions may be - * called normally during the update, including functions that read the scope. - * Updates may be nested for the same scope. + * `sentry_scope_end_write` call on the same thread. Scope functions may be + * called normally during the batch, including functions that read the scope. + * Writes may be nested for the same scope, but an outermost read batch cannot + * be upgraded to a write batch. + * + * Returns 0 on success or 1 if `scope` is NULL, allocation fails, or a read + * batch would need upgrading. Each successful call must be matched by + * `sentry_scope_end_write` before freeing the scope. */ -SENTRY_API void sentry_scope_begin_update(sentry_scope_t *scope); +SENTRY_API int sentry_scope_begin_write(sentry_scope_t *scope); /** - * Ends a batch update begun by `sentry_scope_begin_update`. + * Ends a write batch begun by `sentry_scope_begin_write`. * * The outermost call releases the scope write lock and flushes any pending - * global scope changes to the backend. + * global scope changes to the backend. This must be called on the + * same thread as the matching begin call. */ -SENTRY_API void sentry_scope_end_update(sentry_scope_t *scope); +SENTRY_API void sentry_scope_end_write(sentry_scope_t *scope); /** * Frees a scope created via `sentry_scope_new`, `sentry_scope_clone`, or diff --git a/src/sentry_scope.c b/src/sentry_scope.c index 7f62db094..957672f39 100644 --- a/src/sentry_scope.c +++ b/src/sentry_scope.c @@ -30,8 +30,6 @@ struct sentry_scope_data_s { sentry_rwlock_t rwlock; - uint64_t update_owner; - size_t update_depth; sentry_value_t release; sentry_value_t environment; @@ -63,6 +61,13 @@ struct sentry_scope_data_s { bool trace_managed; }; +typedef struct scope_access_s { + sentry_scope_data_t *data; + size_t depth; + bool write; + struct scope_access_s *next; +} scope_access_t; + static bool g_scope_initialized = false; static sentry_scope_t g_scope = { 0 }; static sentry_scope_data_t g_scope_data = { 0 }; @@ -70,12 +75,10 @@ static bool g_scope_idle_initialized = false; static sentry_cond_t g_scope_idle; #ifdef _MSC_VER static __declspec(thread) size_t g_scope_depth; -static __declspec(thread) size_t g_scope_update_depth; -static __declspec(thread) char g_scope_update_token; +static __declspec(thread) scope_access_t *g_scope_access; #else static __thread size_t g_scope_depth; -static __thread size_t g_scope_update_depth; -static __thread char g_scope_update_token; +static __thread scope_access_t *g_scope_access; #endif #ifdef SENTRY__MUTEX_INIT_DYN SENTRY__MUTEX_INIT_DYN(g_lock) @@ -83,32 +86,28 @@ SENTRY__MUTEX_INIT_DYN(g_lock) static sentry_mutex_t g_lock = SENTRY__MUTEX_INIT; #endif -static uint64_t -scope_update_token(void) -{ - return (uint64_t)(uintptr_t)&g_scope_update_token; -} - -static bool -scope_data_is_updating(const sentry_scope_data_t *data) +static scope_access_t * +scope_data_get_access(const sentry_scope_data_t *data) { #ifndef SENTRY_PLATFORM_WINDOWS // avoid dynamic TLS access during signal handling if (!sentry__block_for_signal_handler()) { - return false; + return NULL; } #endif - if (!g_scope_update_depth) { - return false; + for (scope_access_t *access = g_scope_access; access; + access = access->next) { + if (access->data == data) { + return access; + } } - return sentry__atomic_fetch_u64((uint64_t *)&data->update_owner) - == scope_update_token(); + return NULL; } static bool scope_data_read_lock(const sentry_scope_data_t *data) { - if (scope_data_is_updating(data)) { + if (scope_data_get_access(data)) { return false; } sentry__rwlock_read_lock((sentry_rwlock_t *)&data->rwlock); @@ -126,7 +125,9 @@ scope_data_read_unlock(const sentry_scope_data_t *data, bool locked) static bool scope_data_write_lock(sentry_scope_data_t *data) { - if (scope_data_is_updating(data)) { + scope_access_t *access = scope_data_get_access(data); + assert(!access || access->write); + if (access && access->write) { return false; } sentry__rwlock_write_lock(&data->rwlock); @@ -601,7 +602,8 @@ static bool scope_should_flush(sentry_scope_t *scope, bool flush) { sentry__mutex_lock(&scope->observers_lock); - if (scope->is_notifying > 0 || scope_data_is_updating(scope->data)) { + scope_access_t *access = scope_data_get_access(scope->data); + if (scope->is_notifying > 0 || (access && access->write)) { // defer the flush requested by a reentrant or batched scope change scope->pending_flush = flush || scope->pending_flush; flush = false; @@ -771,53 +773,102 @@ sentry_scope_new(void) return scope; } -void -sentry_scope_begin_update(sentry_scope_t *scope) +static int +scope_begin(const sentry_scope_t *scope, bool write) { if (!scope) { - return; + return 1; } sentry_scope_data_t *data = scope->data; - if (scope_data_is_updating(data)) { - data->update_depth++; - return; + scope_access_t *access = scope_data_get_access(data); + if (access) { + if (write && !access->write) { + return 1; + } + access->depth++; + return 0; } - lock_scope_notify(scope); - sentry__rwlock_write_lock(&data->rwlock); - data->update_depth = 1; - sentry__atomic_store_u64(&data->update_owner, scope_update_token()); - g_scope_update_depth++; + access = SENTRY_MAKE(scope_access_t); + if (!access) { + return 1; + } + + if (write) { + lock_scope_notify((sentry_scope_t *)scope); + sentry__rwlock_write_lock(&data->rwlock); + } else { + sentry__rwlock_read_lock(&data->rwlock); + } + access->data = data; + access->depth = 1; + access->write = write; + access->next = g_scope_access; + g_scope_access = access; + return 0; } -void -sentry_scope_end_update(sentry_scope_t *scope) +int +sentry_scope_begin_read(const sentry_scope_t *scope) +{ + return scope_begin(scope, false); +} + +int +sentry_scope_begin_write(sentry_scope_t *scope) +{ + return scope_begin(scope, true); +} + +static void +scope_end(const sentry_scope_t *scope) { if (!scope) { return; } - sentry_scope_data_t *data = scope->data; - if (!scope_data_is_updating(data)) { + scope_access_t **link = &g_scope_access; + while (*link && (*link)->data != scope->data) { + link = &(*link)->next; + } + scope_access_t *access = *link; + if (!access) { assert(false); return; } - if (--data->update_depth > 0) { + if (--access->depth > 0) { return; } - sentry__atomic_store_u64(&data->update_owner, 0); - g_scope_update_depth--; - sentry__rwlock_write_unlock(&data->rwlock); + *link = access->next; + bool write = access->write; + sentry_free(access); + if (!write) { + sentry__rwlock_read_unlock(&scope->data->rwlock); + return; + } + sentry__rwlock_write_unlock(&scope->data->rwlock); - bool flush = scope_should_flush(scope, scope == &g_scope); - unlock_scope_notify(scope); + bool flush = scope_should_flush((sentry_scope_t *)scope, scope == &g_scope); + unlock_scope_notify((sentry_scope_t *)scope); if (flush && scope == &g_scope) { flush_scope(); } } +void +sentry_scope_end_read(const sentry_scope_t *scope) +{ + scope_end(scope); +} + +void +sentry_scope_end_write(sentry_scope_t *scope) +{ + scope_end(scope); +} + void sentry_scope_free(sentry_scope_t *scope) { diff --git a/tests/unit/test_scope.c b/tests/unit/test_scope.c index 70cde6d70..7b92c67c6 100644 --- a/tests/unit/test_scope.c +++ b/tests/unit/test_scope.c @@ -35,24 +35,26 @@ scope_value_get_length(scope_value_getter_t get, const sentry_scope_t *scope) return length; } -SENTRY_TEST(scope_update_nested) +SENTRY_TEST(scope_write_nested) { sentry_scope_t *scope = sentry_scope_new(); TEST_ASSERT(!!scope); - sentry_scope_begin_update(scope); + TEST_ASSERT(sentry_scope_begin_write(scope) == 0); sentry_scope_set_tag(scope, "first", "value"); - sentry_scope_begin_update(scope); + TEST_ASSERT(sentry_scope_begin_write(scope) == 0); sentry_scope_set_tag(scope, "second", "value"); + TEST_ASSERT(sentry_scope_begin_read(scope) == 0); TEST_CHECK(scope_value_get_length(sentry__scope_load_tags, scope) == 2); - sentry_scope_end_update(scope); + sentry_scope_end_read(scope); + sentry_scope_end_write(scope); sentry_scope_set_environment(scope, "production"); sentry_value_t environment = sentry__scope_ref_environment(scope); TEST_CHECK_STRING_EQUAL(sentry_value_as_string(environment), "production"); sentry_value_decref(environment); - sentry_scope_end_update(scope); + sentry_scope_end_write(scope); TEST_CHECK(scope_value_get_length(sentry__scope_load_tags, scope) == 2); sentry_scope_free(scope); @@ -62,38 +64,49 @@ typedef struct { sentry_scope_t *scope; sentry_waitable_flag_t started; sentry_waitable_flag_t finished; -} scope_update_thread_t; + sentry_value_t environment; + sentry_value_t tag; +} scope_access_thread_t; SENTRY_THREAD_FN -read_scope_during_update(void *data) +read_scope_batch(void *data) { - scope_update_thread_t *state = data; + scope_access_thread_t *state = data; sentry__waitable_flag_set(&state->started); - sentry_value_decref(sentry__scope_load_tags(state->scope)); + TEST_ASSERT(sentry_scope_begin_read(state->scope) == 0); + state->environment = sentry__scope_ref_environment(state->scope); + state->tag = scope_value_get_by_key( + sentry__scope_load_tags, state->scope, "version"); + sentry_scope_end_read(state->scope); sentry__waitable_flag_set(&state->finished); return 0; } -SENTRY_TEST(scope_update_exclusive) +SENTRY_TEST(scope_write_exclusive) { sentry_scope_t *scope = sentry_scope_new(); TEST_ASSERT(!!scope); - scope_update_thread_t state = { .scope = scope }; + scope_access_thread_t state = { .scope = scope }; sentry__waitable_flag_init(&state.started); sentry__waitable_flag_init(&state.finished); - sentry_scope_begin_update(scope); + TEST_ASSERT(sentry_scope_begin_write(scope) == 0); + sentry_scope_set_environment(scope, "new"); sentry_threadid_t thread; sentry__thread_init(&thread); - TEST_ASSERT( - !sentry__thread_spawn(&thread, read_scope_during_update, &state)); + TEST_ASSERT(!sentry__thread_spawn(&thread, read_scope_batch, &state)); TEST_ASSERT(sentry__waitable_flag_wait(&state.started, 1000)); TEST_CHECK(!sentry__waitable_flag_wait(&state.finished, 10)); - sentry_scope_end_update(scope); + sentry_scope_set_tag(scope, "version", "new"); + sentry_scope_end_write(scope); TEST_CHECK(sentry__waitable_flag_wait(&state.finished, 1000)); sentry__thread_join(thread); + TEST_CHECK_STRING_EQUAL(sentry_value_as_string(state.environment), "new"); + TEST_CHECK_STRING_EQUAL(sentry_value_as_string(state.tag), "new"); + sentry_value_decref(state.environment); + sentry_value_decref(state.tag); sentry_scope_free(scope); } @@ -2125,11 +2138,17 @@ SENTRY_TEST(scope_observer_deferred_flush) observer_data.total_flush_count = 0; observer_data.nested_flush_count = 0; sentry_scope_t *scope = sentry__scope_getref(); - sentry_scope_begin_update(scope); + TEST_ASSERT(sentry_scope_begin_write(scope) == 0); sentry_scope_set_tag(scope, "batched", "value"); TEST_CHECK_INT_EQUAL(observer_data.nested_flush_count, 0); TEST_CHECK_INT_EQUAL(observer_data.total_flush_count, 0); - sentry_scope_end_update(scope); + TEST_ASSERT(sentry_scope_begin_read(scope) == 0); + sentry_scope_end_read(scope); + TEST_CHECK_INT_EQUAL(observer_data.total_flush_count, 0); + sentry_scope_end_write(scope); + TEST_CHECK_INT_EQUAL(observer_data.total_flush_count, 1); + TEST_ASSERT(sentry_scope_begin_read(scope) == 0); + sentry_scope_end_read(scope); TEST_CHECK_INT_EQUAL(observer_data.total_flush_count, 1); sentry__scope_finish(scope); @@ -3537,3 +3556,116 @@ SENTRY_TEST(scope_clone_keeps_bound_span) sentry_close(); } + +SENTRY_TEST(scope_read_shared) +{ + sentry_scope_t *scope = sentry_scope_new(); + TEST_ASSERT(!!scope); + sentry_scope_set_environment(scope, "old"); + sentry_scope_set_tag(scope, "version", "old"); + + scope_access_thread_t state = { .scope = scope }; + sentry__waitable_flag_init(&state.started); + sentry__waitable_flag_init(&state.finished); + + TEST_ASSERT(sentry_scope_begin_read(scope) == 0); + sentry_threadid_t thread; + sentry__thread_init(&thread); + TEST_ASSERT(!sentry__thread_spawn(&thread, read_scope_batch, &state)); + TEST_CHECK(sentry__waitable_flag_wait(&state.finished, 1000)); + sentry_scope_end_read(scope); + sentry__thread_join(thread); + + TEST_CHECK_STRING_EQUAL(sentry_value_as_string(state.environment), "old"); + TEST_CHECK_STRING_EQUAL(sentry_value_as_string(state.tag), "old"); + sentry_value_decref(state.environment); + sentry_value_decref(state.tag); + sentry_scope_free(scope); +} + +SENTRY_THREAD_FN +write_scope_during_read(void *data) +{ + scope_access_thread_t *state = data; + sentry__waitable_flag_set(&state->started); + TEST_ASSERT(sentry_scope_begin_write(state->scope) == 0); + sentry_scope_set_environment(state->scope, "new"); + sentry_scope_set_tag(state->scope, "version", "new"); + sentry_scope_end_write(state->scope); + sentry__waitable_flag_set(&state->finished); + return 0; +} + +SENTRY_TEST(scope_read_blocks_write) +{ + sentry_scope_t *scope = sentry_scope_new(); + TEST_ASSERT(!!scope); + sentry_scope_set_environment(scope, "old"); + sentry_scope_set_tag(scope, "version", "old"); + + scope_access_thread_t state = { .scope = scope }; + sentry__waitable_flag_init(&state.started); + sentry__waitable_flag_init(&state.finished); + + TEST_ASSERT(sentry_scope_begin_read(scope) == 0); + sentry_value_t environment = sentry__scope_ref_environment(scope); + TEST_CHECK_STRING_EQUAL(sentry_value_as_string(environment), "old"); + sentry_value_decref(environment); + + sentry_threadid_t thread; + sentry__thread_init(&thread); + TEST_ASSERT( + !sentry__thread_spawn(&thread, write_scope_during_read, &state)); + TEST_ASSERT(sentry__waitable_flag_wait(&state.started, 1000)); + TEST_CHECK(!sentry__waitable_flag_wait(&state.finished, 10)); + + TEST_ASSERT(sentry_scope_begin_read(scope) == 0); + sentry_value_t tag + = scope_value_get_by_key(sentry__scope_load_tags, scope, "version"); + TEST_CHECK_STRING_EQUAL(sentry_value_as_string(tag), "old"); + sentry_value_decref(tag); + TEST_CHECK(sentry_scope_begin_write(scope) == 1); + sentry_scope_end_read(scope); + TEST_CHECK(!sentry__waitable_flag_wait(&state.finished, 10)); + + sentry_scope_end_read(scope); + TEST_CHECK(sentry__waitable_flag_wait(&state.finished, 1000)); + sentry__thread_join(thread); + + environment = sentry__scope_ref_environment(scope); + tag = scope_value_get_by_key(sentry__scope_load_tags, scope, "version"); + TEST_CHECK_STRING_EQUAL(sentry_value_as_string(environment), "new"); + TEST_CHECK_STRING_EQUAL(sentry_value_as_string(tag), "new"); + sentry_value_decref(environment); + sentry_value_decref(tag); + sentry_scope_free(scope); +} + +SENTRY_TEST(scope_access_multiple) +{ + sentry_scope_t *first = sentry_scope_new(); + sentry_scope_t *second = sentry_scope_new(); + TEST_ASSERT(!!first && !!second); + + TEST_ASSERT(sentry_scope_begin_write(first) == 0); + TEST_ASSERT(sentry_scope_begin_read(second) == 0); + TEST_ASSERT(sentry_scope_begin_read(first) == 0); + TEST_ASSERT(sentry_scope_begin_write(first) == 0); + sentry_scope_set_tag(first, "key", "value"); + sentry_scope_end_write(first); + sentry_scope_end_read(first); + sentry_scope_end_write(first); + + TEST_CHECK(scope_value_get_length(sentry__scope_load_tags, first) == 1); + TEST_CHECK(scope_value_get_length(sentry__scope_load_tags, second) == 0); + TEST_CHECK(sentry_scope_begin_write(second) == 1); + sentry_scope_end_read(second); + + TEST_ASSERT(sentry_scope_begin_write(second) == 0); + sentry_scope_set_tag(second, "key", "value"); + sentry_scope_end_write(second); + TEST_CHECK(scope_value_get_length(sentry__scope_load_tags, second) == 1); + + sentry_scope_free(first); + sentry_scope_free(second); +} diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 55cabd710..b42dbc9e3 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -365,6 +365,7 @@ XX(rwlock_write_blocks_writer) XX(sampling_before_send) XX(sampling_decision) XX(sampling_transaction) +XX(scope_access_multiple) XX(scope_attachments) XX(scope_bind_span) XX(scope_bind_span_or_transaction_not_both) @@ -413,6 +414,8 @@ XX(scope_observer_transaction) XX(scope_observer_user) XX(scope_ownership) XX(scope_propagation_context) +XX(scope_read_blocks_write) +XX(scope_read_shared) XX(scope_rebind_same_object) XX(scope_release) XX(scope_remove_fingerprint_capture) @@ -423,10 +426,10 @@ XX(scope_tags) XX(scope_tags_flush) XX(scope_transaction) XX(scope_update_context) -XX(scope_update_exclusive) -XX(scope_update_nested) XX(scope_user) XX(scope_user_id) +XX(scope_write_exclusive) +XX(scope_write_nested) XX(scoped_txn) XX(sentry__value_span_new_requires_unfinished_parent) XX(serialize_envelope) From 2c04beea2e83ab0c40cb9bfb3c80aaa3f95ff889 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Sat, 19 Sep 2026 21:45:19 +0200 Subject: [PATCH 3/3] fix tsan (acutest global data race) --- tests/unit/test_scope.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/tests/unit/test_scope.c b/tests/unit/test_scope.c index 7b92c67c6..9563ad65f 100644 --- a/tests/unit/test_scope.c +++ b/tests/unit/test_scope.c @@ -66,6 +66,7 @@ typedef struct { sentry_waitable_flag_t finished; sentry_value_t environment; sentry_value_t tag; + int result; } scope_access_thread_t; SENTRY_THREAD_FN @@ -73,11 +74,13 @@ read_scope_batch(void *data) { scope_access_thread_t *state = data; sentry__waitable_flag_set(&state->started); - TEST_ASSERT(sentry_scope_begin_read(state->scope) == 0); - state->environment = sentry__scope_ref_environment(state->scope); - state->tag = scope_value_get_by_key( - sentry__scope_load_tags, state->scope, "version"); - sentry_scope_end_read(state->scope); + state->result = sentry_scope_begin_read(state->scope); + if (state->result == 0) { + state->environment = sentry__scope_ref_environment(state->scope); + state->tag = scope_value_get_by_key( + sentry__scope_load_tags, state->scope, "version"); + sentry_scope_end_read(state->scope); + } sentry__waitable_flag_set(&state->finished); return 0; } @@ -103,6 +106,7 @@ SENTRY_TEST(scope_write_exclusive) sentry_scope_end_write(scope); TEST_CHECK(sentry__waitable_flag_wait(&state.finished, 1000)); sentry__thread_join(thread); + TEST_CHECK_INT_EQUAL(state.result, 0); TEST_CHECK_STRING_EQUAL(sentry_value_as_string(state.environment), "new"); TEST_CHECK_STRING_EQUAL(sentry_value_as_string(state.tag), "new"); sentry_value_decref(state.environment); @@ -3576,6 +3580,7 @@ SENTRY_TEST(scope_read_shared) sentry_scope_end_read(scope); sentry__thread_join(thread); + TEST_CHECK_INT_EQUAL(state.result, 0); TEST_CHECK_STRING_EQUAL(sentry_value_as_string(state.environment), "old"); TEST_CHECK_STRING_EQUAL(sentry_value_as_string(state.tag), "old"); sentry_value_decref(state.environment); @@ -3588,10 +3593,12 @@ write_scope_during_read(void *data) { scope_access_thread_t *state = data; sentry__waitable_flag_set(&state->started); - TEST_ASSERT(sentry_scope_begin_write(state->scope) == 0); - sentry_scope_set_environment(state->scope, "new"); - sentry_scope_set_tag(state->scope, "version", "new"); - sentry_scope_end_write(state->scope); + state->result = sentry_scope_begin_write(state->scope); + if (state->result == 0) { + sentry_scope_set_environment(state->scope, "new"); + sentry_scope_set_tag(state->scope, "version", "new"); + sentry_scope_end_write(state->scope); + } sentry__waitable_flag_set(&state->finished); return 0; } @@ -3632,6 +3639,7 @@ SENTRY_TEST(scope_read_blocks_write) TEST_CHECK(sentry__waitable_flag_wait(&state.finished, 1000)); sentry__thread_join(thread); + TEST_CHECK_INT_EQUAL(state.result, 0); environment = sentry__scope_ref_environment(scope); tag = scope_value_get_by_key(sentry__scope_load_tags, scope, "version"); TEST_CHECK_STRING_EQUAL(sentry_value_as_string(environment), "new"); @@ -3656,10 +3664,10 @@ SENTRY_TEST(scope_access_multiple) sentry_scope_end_read(first); sentry_scope_end_write(first); - TEST_CHECK(scope_value_get_length(sentry__scope_load_tags, first) == 1); TEST_CHECK(scope_value_get_length(sentry__scope_load_tags, second) == 0); TEST_CHECK(sentry_scope_begin_write(second) == 1); sentry_scope_end_read(second); + TEST_CHECK(scope_value_get_length(sentry__scope_load_tags, first) == 1); TEST_ASSERT(sentry_scope_begin_write(second) == 0); sentry_scope_set_tag(second, "key", "value");