Skip to content

test: concurrent page allocator - #2035

Merged
jpnurmi merged 3 commits into
masterfrom
jpnurmi/test/concurrent-page-allocator
Aug 31, 2026
Merged

test: concurrent page allocator#2035
jpnurmi merged 3 commits into
masterfrom
jpnurmi/test/concurrent-page-allocator

Conversation

@jpnurmi

@jpnurmi jpnurmi commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

In #974, the originally reported crash was 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

Ref: #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
@jpnurmi
jpnurmi merged commit a0612ef into master Aug 31, 2026
113 of 114 checks passed
@jpnurmi
jpnurmi deleted the jpnurmi/test/concurrent-page-allocator branch August 31, 2026 14:26
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.

2 participants