fix: introduce level-triggered waitable flag to trigger batcher... by supervacuus · Pull Request #1558 · getsentry/sentry-native
…meaningless than before... we are not only bounded by the last flush, but since we now trigger with almost immediate wake on essentially any count in the buffer... we can get requests as high as the number of items we send. The important part is the total count and that one has been stable throughout.
supervacuus
deleted the
fix/introduce_level-triggered_wait_flag
branch
jpnurmi added a commit that referenced this pull request
Mar 6, 2026The two-phase shutdown (begin/wait) was designed to signal both the logs and metrics threads in parallel before joining either. With #1558 replacing the condvar with a level-triggered waitable flag, thread wake latency drops to sub-millisecond (futex/WaitOnAddress), making the parallel signaling unnecessary. Merging into a single function also fixes a race where a concurrent sentry_init() could replace g_batcher between the signal and join steps, causing the new batcher to be shut down instead of the old one. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jpnurmi added a commit that referenced this pull request
Mar 9, 2026…unting (#1556) * test(logs): add stress test for batcher reinit hang Spawns 8 producer threads continuously logging while the main thread does 10 cycles of sentry_init/sentry_close. Reproduces a condvar corruption hang in the batcher under TSan. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(batcher): replace static global with dynamic allocation and refcounting The static g_batcher persisted across init/close cycles, causing condvar corruption when sentry__batcher_startup re-initialized the condvar while old threads still used it. Dynamic allocation with refcounting solves both the condvar corruption and the lifetime problem. Introduces sentry_batcher_ref_t with acquire/release/swap API that encapsulates a spinlock to make concurrent access from producer threads and shutdown safe. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(batcher): release old batcher on swap in startup When the previous startup failed to spawn its thread, shutdown_begin returns false and shutdown_wait is never called. The old batcher stays in g_batcher and leaks when startup swaps it out without releasing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(batcher): NULL-guard batcher in shutdown_wait Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Update CHANGELOG.md * fix(batcher): drain buffer items in release to prevent leaks A producer that acquired a ref before shutdown could enqueue items after the final flush. These items were leaked when the batcher was freed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(batcher): use SENTRY_MAKE macro for allocation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(batcher): destroy condvar before freeing batcher memory Add sentry__cond_destroy for all platforms (pthread_cond_destroy on POSIX, CloseHandle on pre-Vista Windows, no-op on Vista+) and call it in sentry__batcher_release. Without this, each SDK re-initialization leaks kernel handles on pre-Vista Windows and violates POSIX which requires pthread_cond_destroy before freeing memory containing a pthread_cond_t. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Revert "fix(batcher): destroy condvar before freeing batcher memory" This reverts commit 467bfb5ecb2c4528e1048dab1c467203fdc29cdf. * refactor(batcher): merge two-phase shutdown into single function The two-phase shutdown (begin/wait) was designed to signal both the logs and metrics threads in parallel before joining either. With #1558 replacing the condvar with a level-triggered waitable flag, thread wake latency drops to sub-millisecond (futex/WaitOnAddress), making the parallel signaling unnecessary. Merging into a single function also fixes a race where a concurrent sentry_init() could replace g_batcher between the signal and join steps, causing the new batcher to be shut down instead of the old one. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test(metrics): add reinit stress test Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: speed up reinit stress tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(batcher): use lock-free peek in crash-safe flush path The crash-safe flush functions called sentry__batcher_acquire, which spins on a spinlock. If the crash occurs on the thread holding that lock, the signal handler deadlocks. Replace with a new lock-free sentry__batcher_peek that reads ref->ptr via atomic load without taking the spinlock or bumping the refcount (safe because the process is dying). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(batcher): use pointer-width atomics in batcher_peek for Windows On Windows x64, `long` is 32-bit but pointers are 64-bit. Use InterlockedCompareExchangePointer on Windows and __atomic_load elsewhere to avoid truncating the pointer. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs(batcher): add header comments for ref management functions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(batcher): fix race between force flush and reinit force_flush_begin and force_flush_wait each acquired the batcher independently. A concurrent sentry_init could swap the batcher between the two calls, causing wait to flush the new empty batcher while the original data is lost. Fix by returning an opaque token (the acquired batcher ref) from begin and passing it to wait, ensuring both operate on the same instance. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Update src/sentry_logs.c Co-authored-by: Mischan Toosarani-Hausberger <mischan@abovevacant.com> * Update sentry__metrics_startup * docs(batcher): clarify ownership semantics of options refs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Mischan Toosarani-Hausberger <mischan@abovevacant.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters