Skip to content

test: concurrent page allocator - #2035

Open
jpnurmi wants to merge 3 commits into
masterfrom
jpnurmi/test/concurrent-page-allocator
Open

test: concurrent page allocator#2035
jpnurmi wants to merge 3 commits into
masterfrom
jpnurmi/test/concurrent-page-allocator

Conversation

@jpnurmi

@jpnurmi jpnurmi commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

The crashes in #974 were likely caused by the old Unix page allocator spinlock, whose unlock lacked release semantics. On ARM64 this could corrupt allocator state and produce overlapping allocations; sentry_value_set_by_key_n was where the resulting object corruption surfaced.

The synchronization was corrected in #1446. This PR adds the missing regression test: multiple threads allocate concurrently, write unique markers, and verify that every allocation remains independent.

Temporarily restoring the non-atomic pre-#1446 unlock reproduces the race under TSAN, while the corrected implementation passes:

WARNING: ThreadSanitizer: data race
  Write of size 4 by thread T7:
    #0 sentry__page_allocator_alloc src/sentry_unix_pageallocator.c:130

  Previous atomic read of size 4 by thread T3:
    #0 sentry__page_allocator_alloc src/sentry_unix_pageallocator.c:98

  Location is global 'g_lock' of size 4

SUMMARY: ThreadSanitizer: data race in sentry__page_allocator_alloc

https://github.com/getsentry/sentry-native/actions/runs/33185811927/job/98898403787?pr=2035

Close: #974

Exercise the page allocator from synchronized threads and verify that
allocations remain independent. This exposes missing spinlock ordering
under ThreadSanitizer.
Partially revert the spinlock changes from
eecc7fa (#1446) by restoring the
non-atomic unlock.

Together with the concurrent page allocator test in f3ad564, this makes
ThreadSanitizer report the missing synchronization on g_lock and the
resulting races in the allocator state.
@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.36%. Comparing base (719803e) to head (62bc7ce).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2035      +/-   ##
==========================================
+ Coverage   74.21%   74.36%   +0.15%     
==========================================
  Files         104      104              
  Lines       26627    26627              
  Branches     4844     4844              
==========================================
+ Hits        19762    19802      +40     
+ Misses       5519     5477      -42     
- Partials     1346     1348       +2     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jpnurmi
jpnurmi marked this pull request as ready for review August 28, 2026 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Android NDK Segfault crash related to sentry_value_set_by_key_n

1 participant