[WIP] perf: use rwlock for scope data access by jpnurmi · Pull Request #1877 · getsentry/sentry-native

jpnurmi · 2026-07-16T13:02:32Z

Warning

WIP 🚧🔨⏳⛔

Mostly curious what the AI review bots have to say for now...

Allow concurrent scope data readers.

TODO

sentry-playstation

sentry_backend_ps.cpp(88,51): error : member access into incomplete type 'sentry_attachment_t' (aka 'sentry_attachment_s') [C:\github_runners\vie_runner_7\_work\sentry-playstation\sentry-playstation\build\ps5\_deps\sentry-native-build\sentry.vcxproj]
     88 |         const sentry_path_t *filename = attachment->filename ? attachment->filename : attachment->path;
        |                                                   ^
  sentry_backend_ps.h(6,16): note: forward declaration of 'sentry_attachment_s'
      6 | typedef struct sentry_attachment_s sentry_attachment_t;
        |                ^
sentry_backend_ps.cpp(88,74): error : member access into incomplete type 'sentry_attachment_t' (aka 'sentry_attachment_s') [C:\github_runners\vie_runner_7\_work\sentry-playstation\sentry-playstation\build\ps5\_deps\sentry-native-build\sentry.vcxproj]
     88 |         const sentry_path_t *filename = attachment->filename ? attachment->filename : attachment->path;
        |                                                                          ^
  sentry_backend_ps.h(6,16): note: forward declaration of 'sentry_attachment_s'
      6 | typedef struct sentry_attachment_s sentry_attachment_t;
        |                ^
sentry_backend_ps.cpp(88,97): error : member access into incomplete type 'sentry_attachment_t' (aka 'sentry_attachment_s') [C:\github_runners\vie_runner_7\_work\sentry-playstation\sentry-playstation\build\ps5\_deps\sentry-native-build\sentry.vcxproj]
     88 |         const sentry_path_t *filename = attachment->filename ? attachment->filename : attachment->path;
        |                                                                                                 ^
  sentry_backend_ps.h(6,16): note: forward declaration of 'sentry_attachment_s'
      6 | typedef struct sentry_attachment_s sentry_attachment_t;
        |                ^
sentry_backend_ps.cpp(94,23): error : member access into incomplete type 'sentry_attachment_t' (aka 'sentry_attachment_s') [C:\github_runners\vie_runner_7\_work\sentry-playstation\sentry-playstation\build\ps5\_deps\sentry-native-build\sentry.vcxproj]
     94 |         if (attachment->buf) {
        |                       ^
  sentry_backend_ps.h(6,16): note: forward declaration of 'sentry_attachment_s'
      6 | typedef struct sentry_attachment_s sentry_attachment_t;
        |                ^
sentry_backend_ps.cpp(95,27): error : member access into incomplete type 'sentry_attachment_t' (aka 'sentry_attachment_s') [C:\github_runners\vie_runner_7\_work\sentry-playstation\sentry-playstation\build\ps5\_deps\sentry-native-build\sentry.vcxproj]
     95 |             if (attachment->buf_len == 0) {
        |                           ^
  sentry_backend_ps.h(6,16): note: forward declaration of 'sentry_attachment_s'
      6 | typedef struct sentry_attachment_s sentry_attachment_t;
        |                ^
sentry_backend_ps.cpp(102,27): error : member access into incomplete type 'sentry_attachment_t' (aka 'sentry_attachment_s') [C:\github_runners\vie_runner_7\_work\sentry-playstation\sentry-playstation\build\ps5\_deps\sentry-native-build\sentry.vcxproj]
    102 |                 attachment->buf, attachment->buf_len, filename->path);
        |                           ^
  sentry_backend_ps.h(6,16): note: forward declaration of 'sentry_attachment_s'
      6 | typedef struct sentry_attachment_s sentry_attachment_t;
        |                ^
sentry_backend_ps.cpp(102,44): error : member access into incomplete type 'sentry_attachment_t' (aka 'sentry_attachment_s') [C:\github_runners\vie_runner_7\_work\sentry-playstation\sentry-playstation\build\ps5\_deps\sentry-native-build\sentry.vcxproj]
    102 |                 attachment->buf, attachment->buf_len, filename->path);
        |                                            ^
  sentry_backend_ps.h(6,16): note: forward declaration of 'sentry_attachment_s'
      6 | typedef struct sentry_attachment_s sentry_attachment_t;
        |                ^
sentry_backend_ps.cpp(110,27): error : member access into incomplete type 'sentry_attachment_t' (aka 'sentry_attachment_s') [C:\github_runners\vie_runner_7\_work\sentry-playstation\sentry-playstation\build\ps5\_deps\sentry-native-build\sentry.vcxproj]
    110 |                 attachment->path->path);
        |                           ^
  sentry_backend_ps.h(6,16): note: forward declaration of 'sentry_attachment_s'
      6 | typedef struct sentry_attachment_s sentry_attachment_t;
        |                ^
sentry_backend_ps.cpp(328,14): error : no member named 'add_attachment_func' in 'sentry_backend_s' [C:\github_runners\vie_runner_7\_work\sentry-playstation\sentry-playstation\build\ps5\_deps\sentry-native-build\sentry.vcxproj]
    328 |     backend->add_attachment_func = ps_backend_add_attachment;
        |     ~~~~~~~  ^
sentry_backend_ps.cpp(329,14): error : no member named 'remove_attachment_func' in 'sentry_backend_s' [C:\github_runners\vie_runner_7\_work\sentry-playstation\sentry-playstation\build\ps5\_deps\sentry-native-build\sentry.vcxproj]
    329 |     backend->remove_attachment_func = ps_backend_remove_attachment;
        |     ~~~~~~~  ^

sentry-switch

sentry_core_nx.cpp:74:40: error: no member named 'contexts' in 'sentry_scope_s'
 74 |         sentry_value_set_by_key(scope->contexts, "device", device);
    |                                 ~~~~~  ^

sentry_backend_nx.cpp:68:48: error: no member named 'user' in 'sentry_scope_s'
 68 |             if (sentry_value_get_length(scope->user) == 0) {
    |                                         ~~~~~  ^

sentry-xbox

sentry_core_xbox.cpp(64,40): error C2039: 'contexts': is not a member of 'sentry_scope_s' [sentry-native-build\sentry.vcxproj]
    sentry_scope.h(58,8): see declaration of 'sentry_scope_s'

Before

tests/benchmark.py::test_benchmark_logs[1] PASSED
Min 0.002ms, Max 0.002ms, Mean 0.002ms, Median 0.002ms, CPU 0.002ms                                                                                                                                                                                                                                      [ 25%]
tests/benchmark.py::test_benchmark_logs[8] PASSED
Min 0.026ms, Max 0.026ms, Mean 0.026ms, Median 0.026ms, CPU 0.008ms                                                                                                                                                                                                                                      [ 50%]
tests/benchmark.py::test_benchmark_logs[16] PASSED
Min 0.067ms, Max 0.067ms, Mean 0.067ms, Median 0.067ms, CPU 0.010ms                                                                                                                                                                                                                                      [ 75%]
tests/benchmark.py::test_benchmark_logs[32] PASSED
Min 0.121ms, Max 0.121ms, Mean 0.121ms, Median 0.121ms, CPU 0.010ms

After

tests/benchmark.py::test_benchmark_logs[1] PASSED
Min 0.004ms, Max 0.004ms, Mean 0.004ms, Median 0.004ms, CPU 0.004ms                                                                                                                                                                                                                                      [ 25%]
tests/benchmark.py::test_benchmark_logs[8] PASSED
Min 0.011ms, Max 0.011ms, Mean 0.011ms, Median 0.011ms, CPU 0.011ms                                                                                                                                                                                                                                      [ 50%]
tests/benchmark.py::test_benchmark_logs[16] PASSED
Min 0.024ms, Max 0.024ms, Mean 0.024ms, Median 0.024ms, CPU 0.022ms                                                                                                                                                                                                                                      [ 75%]
tests/benchmark.py::test_benchmark_logs[32] PASSED
Min 0.048ms, Max 0.048ms, Mean 0.048ms, Median 0.048ms, CPU 0.030ms

Close: #1862

github-actions · 2026-07-16T13:03:04Z

	Fails
🚫	Please consider adding a changelog entry for the next release.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

### Features

- use rwlock for scope data access ([#1877](https://github.com/getsentry/sentry-native/pull/1877))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description or adding a skip-changelog label.

Generated by 🚫 dangerJS against 135c53c

codecov · 2026-07-16T13:25:30Z

Codecov Report

❌ Patch coverage is 94.09283% with 70 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (jpnurmi/ref/attachments@a6dd4ac). Learn more about missing BASE report.

Additional details and impacted files

@@                    Coverage Diff                     @@
##             jpnurmi/ref/attachments    #1877   +/-   ##
==========================================================
  Coverage                           ?   75.05%           
==========================================================
  Files                              ?      104           
  Lines                              ?    26840           
  Branches                           ?     4853           
==========================================================
  Hits                               ?    20145           
  Misses                             ?     5367           
  Partials                           ?     1328

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

sentry · 2026-08-12T06:37:19Z

+        sentry__cond_wait(&g_scope_idle, &g_lock);
+    }
    if (g_scope_initialized) {
        g_scope_initialized = false;
-        cleanup_scope(&g_scope);
+        cleanup_global_data(g_scope.data);
+        g_scope.data = NULL;
+        cleanup_observers(&g_scope);
+        sentry__mutex_free(&g_scope.observers_lock);
    }
    sentry__mutex_unlock(&g_lock);
 }


Bug: The condition variable g_scope_idle is initialized but never destroyed, causing a kernel handle leak on some Windows platforms.
_{Severity: LOW}

Suggested Fix

Implement a cleanup path for the g_scope_idle condition variable. This can be done by adding a call to a new sentry__cond_free(&g_scope_idle) function within sentry__scope_cleanup(), ensuring it is only called when g_scope_idle_initialized is true. The sentry__cond_free function would need to be implemented to close the relevant handles on Windows.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: src/sentry_scope.c#L1219-L1234 Potential issue: The condition variable `g_scope_idle` is initialized on first use within `sentry__scope_begin()` but is never destroyed. On pre-Vista Windows platforms, the implementation of `sentry__cond_init` allocates kernel handles for a semaphore and an event. Since there is no corresponding cleanup function called during shutdown (e.g., in `sentry__scope_cleanup`), these handles are leaked. This leads to a resource leak on affected Windows versions.

To be fixed with the help of:

fix(sync): Destroy condition variables #2004

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 3 total unresolved issues (including 2 from previous reviews).

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit dcbac0c. Configure here.}

Add a non-recursive sentry_rwlock_t abstraction backed by platform reader/writer locks. Cover static and dynamic initialization, reader sharing, writer exclusion, and mixed read/write stress in unit tests.

Convert the POSIX rwlock operations from statement macros to static inline functions so callers can use them from scoped lock macro expressions.

Add a per-scope rwlock and use scoped read/write locking for simple by-value fields. Keep borrowed scope properties unchanged for now.

Add ref-style scope accessors for user and fingerprint values so callers get a retained reference that was incremented under the scope read lock. Guard user and fingerprint replacement with the scope write lock and keep observer notifications outside the locked section.

Store release, environment, and transaction as sentry_value_t fields with retained scope accessors. Pass value references through observer notifications and add a generic consuming sentry__value_replace helper for scope value replacement.

Move scope payload access behind sentry_scope_data_t helpers so scope-level code no longer reaches through scope->data to individual fields.

Track active scope accesses and delay cleanup until they finish, while releasing the lifetime guard before backend scope flushing.

Register backend attachment observers for the SDK lifetime and remove the attachment callbacks from sentry_backend_t. Backends now react only after an attachment is committed to the scope. This keeps duplicate detection and backend registration in the same serialized scope operation, preventing concurrent additions from registering the same file twice.

jpnurmi changed the title ~~perf: use recursive rwlock for global scope~~ [WIP] perf: use recursive rwlock for global scope Jul 16, 2026

cursor Bot reviewed Jul 16, 2026

View reviewed changes

Comment thread src/sentry_sync.h Outdated

Comment thread src/sentry_sync.h Outdated

cursor Bot reviewed Jul 16, 2026

View reviewed changes

Comment thread src/sentry_sync.h Outdated

Comment thread src/sentry_scope.c

sentry Bot reviewed Jul 17, 2026

View reviewed changes

Comment thread src/sentry_sync.h Outdated

jpnurmi force-pushed the jpnurmi/perf/rwlock branch from 495cd85 to 05fe336 Compare August 7, 2026 14:42

jpnurmi changed the title ~~[WIP] perf: use recursive rwlock for global scope~~ [WIP] perf: use rwlock for scope data access Aug 7, 2026

sentry Bot reviewed Aug 7, 2026

View reviewed changes

Comment thread src/sentry_scope.c Outdated

cursor Bot reviewed Aug 7, 2026

View reviewed changes

Comment thread src/sentry_options.h

Comment thread src/sentry_attachment.c

Comment thread src/backends/sentry_backend_breakpad.cpp

sentry Bot reviewed Aug 10, 2026

View reviewed changes

Comment thread src/sentry_scope.c

Comment thread src/sentry_scope.c

Comment thread src/sentry_attachment.c Outdated

cursor Bot reviewed Aug 10, 2026

View reviewed changes

Comment thread src/sentry_scope.c Outdated

jpnurmi force-pushed the jpnurmi/perf/rwlock branch from 3c7c966 to 4948141 Compare August 12, 2026 04:55

jpnurmi changed the base branch from master to jpnurmi/ref/attachments August 12, 2026 04:56

jpnurmi force-pushed the jpnurmi/perf/rwlock branch from 4948141 to 0b0e77c Compare August 12, 2026 06:33

sentry Bot reviewed Aug 12, 2026

View reviewed changes

cursor Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread src/sentry_scope.c Outdated

Comment thread src/sentry_scope.c

sentry Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread src/sentry_scope.c Outdated

jpnurmi force-pushed the jpnurmi/perf/rwlock branch from dab304d to b899738 Compare August 17, 2026 17:52

sentry Bot reviewed Aug 17, 2026

View reviewed changes

Comment thread src/sentry_scope.c Outdated

cursor Bot reviewed Aug 17, 2026

View reviewed changes

Comment thread src/integrations/sentry_integration_wer.c

Comment thread src/sentry_core.c Outdated

jpnurmi mentioned this pull request Aug 18, 2026

ref!: sentry_value-based attachments #1974

Open

jpnurmi force-pushed the jpnurmi/perf/rwlock branch from b899738 to 853172c Compare August 18, 2026 12:26

sentry Bot reviewed Aug 18, 2026

View reviewed changes

Comment thread src/sentry_scope.c

Comment thread src/sentry_scope.c

jpnurmi force-pushed the jpnurmi/perf/rwlock branch from 853172c to dcbac0c Compare August 18, 2026 12:37

cursor Bot reviewed Aug 18, 2026

View reviewed changes

Comment thread src/sentry_scope.c

jpnurmi force-pushed the jpnurmi/perf/rwlock branch from f197136 to 802a09a Compare August 19, 2026 10:32

jpnurmi added 27 commits August 19, 2026 13:27

refactor(scope): hide direct data access

f197880

WIP: observers

ffa73f7

feat(sync): add platform rwlock primitive

56f7f07

Add a non-recursive sentry_rwlock_t abstraction backed by platform reader/writer locks. Cover static and dynamic initialization, reader sharing, writer exclusion, and mixed read/write stress in unit tests.

refactor(sync): make rwlock operations inline

47bba26

Convert the POSIX rwlock operations from statement macros to static inline functions so callers can use them from scoped lock macro expressions.

refactor(scope): guard simple fields with rwlock

aa0be8f

Add a per-scope rwlock and use scoped read/write locking for simple by-value fields. Keep borrowed scope properties unchanged for now.

refactor(scope): store string fields as values

6ff86c0

Store release, environment, and transaction as sentry_value_t fields with retained scope accessors. Pass value references through observer notifications and add a generic consuming sentry__value_replace helper for scope value replacement.

refactor(scope): move locked payload into scope data

9a2b29a

refactor(scope): hide direct scope data access

0828f8f

Move scope payload access behind sentry_scope_data_t helpers so scope-level code no longer reaches through scope->data to individual fields.

ref containers

813b7b0

breadcrumbs

d9ba2a8

trace_context

9e3bbba

propagation_context

e821d6e

dsc

2949eec

span/transaction_object

16b07d8

restore

41717ae

lock observers

768a86a

finally :D

b3a38df

fix formatting

61892ba

fix(scope): Keep global scope alive during access

bc99dba

Track active scope accesses and delay cleanup until they finish, while releasing the lifetime guard before backend scope flushing.

restore locks for attachments

8b02357

clone (cow) attachments guard against concurrent modifications

2629b07

clean up

f70a7c6

fix concurrent init

fbbfaae

breadcrumbs: fix use-after-free

25a7e0c

fix cleanup_observers

abcc49d

jpnurmi force-pushed the jpnurmi/perf/rwlock branch from 802a09a to abcc49d Compare August 19, 2026 11:29

load

135c53c

Uh oh!

Uh oh!

Conversation

jpnurmi commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

WIP 🚧🔨⏳⛔

TODO

Before

After

Uh oh!

github-actions Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Instructions and example for changelog

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sentry Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

jpnurmi Aug 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jpnurmi commented Jul 16, 2026 •

edited

Loading

github-actions Bot commented Jul 16, 2026 •

edited

Loading

codecov Bot commented Jul 16, 2026 •

edited

Loading