From 95affbb31a1283364f377d0800ebfc2c7c86acf4 Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Thu, 30 Jul 2026 14:57:13 +0200 Subject: [PATCH 1/3] replace sleep_ms with virtual time in app hang tests --- src/sentry_app_hang_monitor.c | 58 +++++++++++++++---------- src/sentry_app_hang_monitor.h | 7 +-- src/sentry_utils.c | 61 ++++++++++++++++++++++++++ src/sentry_utils.h | 23 ++++++++++ tests/unit/test_app_hang.c | 82 ++++++++++++++++++++++++++--------- tests/unit/test_utils.c | 16 +++++++ tests/unit/tests.inc | 1 + 7 files changed, 199 insertions(+), 49 deletions(-) diff --git a/src/sentry_app_hang_monitor.c b/src/sentry_app_hang_monitor.c index 6054028547..0e169d4de2 100644 --- a/src/sentry_app_hang_monitor.c +++ b/src/sentry_app_hang_monitor.c @@ -85,6 +85,30 @@ app_hang_capture(uint64_t hang_time_ms, uint64_t tid) return true; } +void +sentry__app_hang_monitor_check(uint64_t *last_fired_heartbeat_ms) +{ + if (!sentry__app_hang_is_active() || sentry__app_hang_is_paused()) { + return; + } + + const sentry_app_hang_latch_t latch = sentry__app_hang_current_latch(); + uint64_t now = sentry__monotonic_time(); + if (!sentry__app_hang_should_capture(latch.last_heartbeat_ms, now, + g_timeout_ms, *last_fired_heartbeat_ms)) { + return; + } + + // Re-check state immediately before stackwalking to avoid reporting while + // crash handling or an explicit pause has disarmed monitoring. + if (!sentry__app_hang_is_active() || sentry__app_hang_is_paused()) { + return; + } + if (app_hang_capture(now - latch.last_heartbeat_ms, latch.target_tid)) { + *last_fired_heartbeat_ms = latch.last_heartbeat_ms; + } +} + SENTRY_THREAD_FN worker(void *arg) { @@ -92,6 +116,10 @@ worker(void *arg) uint64_t last_fired_hb = 0; while (sentry__app_hang_is_active()) { sentry__mutex_lock(&g_wait_mutex); + if (!sentry__app_hang_is_active()) { + sentry__mutex_unlock(&g_wait_mutex); + break; + } sentry__cond_wait_timeout( &g_wait_cond, &g_wait_mutex, SENTRY_APP_HANG_POLL_MS); sentry__mutex_unlock(&g_wait_mutex); @@ -99,29 +127,7 @@ worker(void *arg) if (!sentry__app_hang_is_active()) { break; } - if (sentry__app_hang_is_paused()) { - continue; - } - - const sentry_app_hang_latch_t latch = sentry__app_hang_current_latch(); - uint64_t now = sentry__monotonic_time(); - if (sentry__app_hang_should_capture( - latch.last_heartbeat_ms, now, g_timeout_ms, last_fired_hb)) { - // Bail if disarmed. Keeps duplicate reporting window minimal - if (!sentry__app_hang_is_active()) { - break; - } - if (sentry__app_hang_is_paused()) { - continue; - } - // Only mark this freeze as fired when an event was actually - // captured. A transient stackwalk failure (0 frames) must not - // suppress retries while the thread remains stuck. - if (app_hang_capture( - now - latch.last_heartbeat_ms, latch.target_tid)) { - last_fired_hb = latch.last_heartbeat_ms; - } - } + sentry__app_hang_monitor_check(&last_fired_hb); } return 0; } @@ -186,4 +192,10 @@ sentry__app_hang_monitor_stop(void) { } +void +sentry__app_hang_monitor_check(uint64_t *last_fired_heartbeat_ms) +{ + (void)last_fired_heartbeat_ms; +} + #endif // SENTRY_HAS_THREAD_STACKWALK diff --git a/src/sentry_app_hang_monitor.h b/src/sentry_app_hang_monitor.h index 9ecad56f80..83787c4e54 100644 --- a/src/sentry_app_hang_monitor.h +++ b/src/sentry_app_hang_monitor.h @@ -9,11 +9,7 @@ struct sentry_options_s; // Interval at which the watchdog samples the heartbeat. -#if defined(SENTRY_UNITTEST) -# define SENTRY_APP_HANG_POLL_MS 10 -#else -# define SENTRY_APP_HANG_POLL_MS 500 -#endif +#define SENTRY_APP_HANG_POLL_MS 500 // Smallest timeout the watchdog can resolve meaningfully. A genuine hang fires // somewhere in [timeout, timeout + POLL_MS) depending on the phase between the @@ -25,6 +21,7 @@ struct sentry_options_s; int sentry__app_hang_monitor_start(const struct sentry_options_s *options); void sentry__app_hang_monitor_stop(void); +void sentry__app_hang_monitor_check(uint64_t *last_fired_heartbeat_ms); // Test hook: overrides the platform thread stackwalker used by the watchdog. typedef size_t (*sentry__app_hang_stackwalk_fn)( diff --git a/src/sentry_utils.c b/src/sentry_utils.c index db255ca782..5e15f0f11d 100644 --- a/src/sentry_utils.c +++ b/src/sentry_utils.c @@ -20,6 +20,67 @@ #include #include +#ifdef SENTRY_UNITTEST +static volatile long g_test_clock_enabled = 0; +static uint64_t g_test_clock_monotonic_ms = 0; +static uint64_t g_test_clock_epoch_usec = 0; + +static uint64_t +add_saturate(uint64_t value, uint64_t increment) +{ + return value > UINT64_MAX - increment ? UINT64_MAX : value + increment; +} + +void +sentry__test_clock_set(uint64_t monotonic_ms, uint64_t epoch_usec) +{ + sentry__atomic_store_u64(&g_test_clock_monotonic_ms, monotonic_ms); + sentry__atomic_store_u64(&g_test_clock_epoch_usec, epoch_usec); + sentry__atomic_store(&g_test_clock_enabled, 1); +} + +void +sentry__test_clock_advance(uint64_t milliseconds) +{ + assert(sentry__atomic_fetch(&g_test_clock_enabled)); + + uint64_t monotonic_ms + = sentry__atomic_fetch_u64(&g_test_clock_monotonic_ms); + uint64_t epoch_usec = sentry__atomic_fetch_u64(&g_test_clock_epoch_usec); + sentry__atomic_store_u64( + &g_test_clock_monotonic_ms, add_saturate(monotonic_ms, milliseconds)); + sentry__atomic_store_u64(&g_test_clock_epoch_usec, + add_saturate(epoch_usec, + milliseconds > UINT64_MAX / 1000 ? UINT64_MAX : milliseconds * 1000)); +} + +void +sentry__test_clock_reset(void) +{ + sentry__atomic_store(&g_test_clock_enabled, 0); + sentry__atomic_store_u64(&g_test_clock_monotonic_ms, 0); + sentry__atomic_store_u64(&g_test_clock_epoch_usec, 0); +} + +bool +sentry__test_clock_is_enabled(void) +{ + return sentry__atomic_fetch(&g_test_clock_enabled) != 0; +} + +uint64_t +sentry__test_clock_monotonic_time(void) +{ + return sentry__atomic_fetch_u64(&g_test_clock_monotonic_ms); +} + +uint64_t +sentry__test_clock_usec_time(void) +{ + return sentry__atomic_fetch_u64(&g_test_clock_epoch_usec); +} +#endif + #ifdef SENTRY_PLATFORM_DARWIN # include #elif defined(SENTRY_PLATFORM_LINUX) && !defined(SENTRY_PLATFORM_ANDROID) diff --git a/src/sentry_utils.h b/src/sentry_utils.h index 775ab80864..af51a2e4b2 100644 --- a/src/sentry_utils.h +++ b/src/sentry_utils.h @@ -164,12 +164,30 @@ char *sentry__dsn_get_minidump_url( */ char *sentry__base64_encode(const char *data, size_t len); +#ifdef SENTRY_UNITTEST +/** + * Overrides SDK time for deterministic unit tests. The clock is process-global, + * so callers must stop all SDK threads before resetting it. + */ +void sentry__test_clock_set(uint64_t monotonic_ms, uint64_t epoch_usec); +void sentry__test_clock_advance(uint64_t milliseconds); +void sentry__test_clock_reset(void); +bool sentry__test_clock_is_enabled(void); +uint64_t sentry__test_clock_monotonic_time(void); +uint64_t sentry__test_clock_usec_time(void); +#endif + /** * Returns the number of microseconds since the unix epoch. */ static inline uint64_t sentry__usec_time(void) { +#ifdef SENTRY_UNITTEST + if (sentry__test_clock_is_enabled()) { + return sentry__test_clock_usec_time(); + } +#endif #ifdef SENTRY_PLATFORM_WINDOWS // Contains a 64-bit value representing the number of 100-nanosecond // intervals since January 1, 1601 (UTC). @@ -198,6 +216,11 @@ sentry__usec_time(void) static inline uint64_t sentry__monotonic_time(void) { +#ifdef SENTRY_UNITTEST + if (sentry__test_clock_is_enabled()) { + return sentry__test_clock_monotonic_time(); + } +#endif #ifdef SENTRY_PLATFORM_WINDOWS static LARGE_INTEGER qpc_frequency = { { 0, 0 } }; diff --git a/tests/unit/test_app_hang.c b/tests/unit/test_app_hang.c index 176e8eee59..2286284173 100644 --- a/tests/unit/test_app_hang.c +++ b/tests/unit/test_app_hang.c @@ -3,6 +3,13 @@ #include "sentry_sync.h" #include "sentry_testsupport.h" #include "sentry_thread_stackwalk.h" +#include "sentry_utils.h" + +static void +set_test_clock(void) +{ + sentry__test_clock_set(1000, 1700000000000000ULL); +} SENTRY_TEST(app_hang_should_capture) { @@ -48,6 +55,7 @@ SENTRY_TEST(app_hang_pause_resumes_on_heartbeat) #if !SENTRY_HAS_THREAD_STACKWALK SKIP_TEST(); #endif + set_test_clock(); sentry__app_hang_latch_reset(); sentry__app_hang_set_active(true); sentry_app_hang_heartbeat(); @@ -64,7 +72,7 @@ SENTRY_TEST(app_hang_pause_resumes_on_heartbeat) TEST_CHECK(l.target_tid == target); TEST_CHECK(l.last_heartbeat_ms == first_heartbeat); - sleep_ms(1); + sentry__test_clock_advance(1); sentry_app_hang_heartbeat(); l = sentry__app_hang_current_latch(); TEST_CHECK(!sentry__app_hang_is_paused()); @@ -73,6 +81,7 @@ SENTRY_TEST(app_hang_pause_resumes_on_heartbeat) sentry__app_hang_latch_reset(); sentry__app_hang_set_active(false); + sentry__test_clock_reset(); } SENTRY_TEST(app_hang_make_event) @@ -145,6 +154,7 @@ SENTRY_TEST(app_hang_monitor_fires) #if !SENTRY_HAS_THREAD_STACKWALK SKIP_TEST(); #endif + set_test_clock(); g_app_hang_seen = 0; g_app_hang_type[0] = '\0'; sentry__app_hang_latch_reset(); @@ -154,20 +164,20 @@ SENTRY_TEST(app_hang_monitor_fires) sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); sentry_options_set_before_send(options, capture_before_send, NULL); sentry_options_set_enable_app_hang_tracking(options, 1); - sentry_options_set_app_hang_timeout(options, 50); + sentry_options_set_app_hang_timeout(options, 1000); sentry_init(options); sentry_app_hang_heartbeat(); - - for (int i = 0; i < 300 && !sentry__atomic_fetch(&g_app_hang_seen); i++) { - sleep_ms(10); - } + sentry__test_clock_advance(1000); + uint64_t last_fired_heartbeat_ms = 0; + sentry__app_hang_monitor_check(&last_fired_heartbeat_ms); TEST_CHECK(sentry__atomic_fetch(&g_app_hang_seen) == 1); TEST_CHECK_STRING_EQUAL(g_app_hang_type, "AppHang"); sentry_close(); sentry__app_hang_monitor_set_stackwalk_fn(NULL); + sentry__test_clock_reset(); } SENTRY_TEST(app_hang_pause_prevents_capture) @@ -175,6 +185,7 @@ SENTRY_TEST(app_hang_pause_prevents_capture) #if !SENTRY_HAS_THREAD_STACKWALK SKIP_TEST(); #endif + set_test_clock(); g_app_hang_seen = 0; g_app_hang_type[0] = '\0'; sentry__app_hang_latch_reset(); @@ -184,35 +195,39 @@ SENTRY_TEST(app_hang_pause_prevents_capture) sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); sentry_options_set_before_send(options, capture_before_send, NULL); sentry_options_set_enable_app_hang_tracking(options, 1); - sentry_options_set_app_hang_timeout(options, 50); + sentry_options_set_app_hang_timeout(options, 1000); sentry_init(options); sentry_app_hang_heartbeat(); sentry_app_hang_pause(); - // Wait beyond the timeout and several poll cycles while the worker is - // paused. - sleep_ms(100); + sentry__test_clock_advance(2000); + uint64_t last_fired_heartbeat_ms = 0; + sentry__app_hang_monitor_check(&last_fired_heartbeat_ms); TEST_CHECK(sentry__atomic_fetch(&g_app_hang_seen) == 0); sentry_app_hang_heartbeat(); - for (int i = 0; i < 300 && !sentry__atomic_fetch(&g_app_hang_seen); i++) { - sleep_ms(10); - } + sentry__test_clock_advance(1000); + sentry__app_hang_monitor_check(&last_fired_heartbeat_ms); TEST_CHECK(sentry__atomic_fetch(&g_app_hang_seen) == 1); TEST_CHECK_STRING_EQUAL(g_app_hang_type, "AppHang"); sentry_close(); sentry__app_hang_monitor_set_stackwalk_fn(NULL); + sentry__test_clock_reset(); } SENTRY_TEST(app_hang_disarm_prevents_capture) { +#if !SENTRY_HAS_THREAD_STACKWALK + SKIP_TEST(); +#endif // Mirrors app_hang_monitor_fires, but disarms after latching. This is the // crash-handler path: once disarmed, the watchdog must not capture an // app-hang even though the latched thread stops heart-beating (so a crash // is never also reported as an app-hang). + set_test_clock(); g_app_hang_seen = 0; g_app_hang_type[0] = '\0'; sentry__app_hang_latch_reset(); @@ -222,27 +237,34 @@ SENTRY_TEST(app_hang_disarm_prevents_capture) sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); sentry_options_set_before_send(options, capture_before_send, NULL); sentry_options_set_enable_app_hang_tracking(options, 1); - sentry_options_set_app_hang_timeout(options, 50); + sentry_options_set_app_hang_timeout(options, 1000); sentry_init(options); sentry_app_hang_heartbeat(); // Simulate entering the crash handler: disable -> never heartbeat again. sentry__app_hang_set_active(false); - // Wait well past several timeout/poll cycles to be sure nothing fires. - for (int i = 0; i < 50; i++) { - sleep_ms(10); - } + sentry__test_clock_advance(2000); + uint64_t last_fired_heartbeat_ms = 0; + sentry__app_hang_monitor_check(&last_fired_heartbeat_ms); TEST_CHECK(sentry__atomic_fetch(&g_app_hang_seen) == 0); sentry_close(); sentry__app_hang_monitor_set_stackwalk_fn(NULL); + sentry__test_clock_reset(); } static long g_real_seen; static long g_real_frames; static volatile long g_keep_spinning; +static sentry_cond_t g_spinner_ready; +#ifdef SENTRY__MUTEX_INIT_DYN +SENTRY__MUTEX_INIT_DYN(spinner_lock) +#else +static sentry_mutex_t spinner_lock = SENTRY__MUTEX_INIT; +#endif +static bool g_spinner_started; static sentry_value_t real_before_send(sentry_value_t event, void *hint, void *data) @@ -266,6 +288,11 @@ spinner(void *arg) { (void)arg; sentry_app_hang_heartbeat(); // latch this thread + SENTRY__MUTEX_INIT_DYN_ONCE(spinner_lock); + sentry__mutex_lock(&spinner_lock); + g_spinner_started = true; + sentry__cond_wake(&g_spinner_ready); + sentry__mutex_unlock(&spinner_lock); while (sentry__atomic_fetch(&g_keep_spinning)) { // busy-wait: alive & sampleable but never heartbeats again -> hung volatile int x = 0; @@ -281,9 +308,13 @@ SENTRY_TEST(app_hang_end_to_end) #if !SENTRY_HAS_THREAD_STACKWALK SKIP_TEST(); #endif + set_test_clock(); g_real_seen = 0; g_real_frames = 0; sentry__atomic_store(&g_keep_spinning, 1); + SENTRY__MUTEX_INIT_DYN_ONCE(spinner_lock); + sentry__cond_init(&g_spinner_ready); + g_spinner_started = false; sentry__app_hang_latch_reset(); sentry__app_hang_monitor_set_stackwalk_fn(NULL); // use the REAL stackwalker @@ -291,15 +322,23 @@ SENTRY_TEST(app_hang_end_to_end) sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); sentry_options_set_before_send(options, real_before_send, NULL); sentry_options_set_enable_app_hang_tracking(options, 1); - sentry_options_set_app_hang_timeout(options, 50); + sentry_options_set_app_hang_timeout(options, 1000); sentry_init(options); sentry_threadid_t t; sentry__thread_spawn(&t, spinner, NULL); - for (int i = 0; i < 500 && !sentry__atomic_fetch(&g_real_seen); i++) { - sleep_ms(10); + sentry__mutex_lock(&spinner_lock); + for (int i = 0; i < 10 && !g_spinner_started; i++) { + sentry__cond_wait_timeout(&g_spinner_ready, &spinner_lock, 100); } + const bool spinner_started = g_spinner_started; + sentry__mutex_unlock(&spinner_lock); + TEST_ASSERT(spinner_started); + + sentry__test_clock_advance(1000); + uint64_t last_fired_heartbeat_ms = 0; + sentry__app_hang_monitor_check(&last_fired_heartbeat_ms); sentry__atomic_store(&g_keep_spinning, 0); sentry__thread_join(t); @@ -309,4 +348,5 @@ SENTRY_TEST(app_hang_end_to_end) sentry_close(); sentry__app_hang_monitor_set_stackwalk_fn(NULL); + sentry__test_clock_reset(); } diff --git a/tests/unit/test_utils.c b/tests/unit/test_utils.c index c9d5d30928..09eb4a1a71 100644 --- a/tests/unit/test_utils.c +++ b/tests/unit/test_utils.c @@ -38,6 +38,22 @@ SENTRY_TEST(iso_time) TEST_CHECK_INT_EQUAL(roundtrip, usec); } +SENTRY_TEST(virtual_clock) +{ + sentry__test_clock_set(1000, 1700000000000000ULL); + + TEST_CHECK(sentry__test_clock_is_enabled()); + TEST_CHECK_UINT64_EQUAL(sentry__monotonic_time(), 1000); + TEST_CHECK_UINT64_EQUAL(sentry__usec_time(), 1700000000000000ULL); + + sentry__test_clock_advance(250); + TEST_CHECK_UINT64_EQUAL(sentry__monotonic_time(), 1250); + TEST_CHECK_UINT64_EQUAL(sentry__usec_time(), 1700000000250000ULL); + + sentry__test_clock_reset(); + TEST_CHECK(!sentry__test_clock_is_enabled()); +} + static void check_url(const sentry_url_t *url) { diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index b88e07453b..8f910bb10d 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -415,6 +415,7 @@ XX(user_feedback_with_null_args) XX(user_report_is_valid) XX(uuid_api) XX(uuid_v4) +XX(virtual_clock) XX(value_attribute) XX(value_bool) XX(value_clone_free_original_first) From 0192a61960d6a3292ee3e3577cada0c249b6c5bc Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Thu, 30 Jul 2026 15:19:00 +0200 Subject: [PATCH 2/3] style --- src/sentry_utils.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/sentry_utils.c b/src/sentry_utils.c index 5e15f0f11d..5f0119eeea 100644 --- a/src/sentry_utils.c +++ b/src/sentry_utils.c @@ -51,7 +51,8 @@ sentry__test_clock_advance(uint64_t milliseconds) &g_test_clock_monotonic_ms, add_saturate(monotonic_ms, milliseconds)); sentry__atomic_store_u64(&g_test_clock_epoch_usec, add_saturate(epoch_usec, - milliseconds > UINT64_MAX / 1000 ? UINT64_MAX : milliseconds * 1000)); + milliseconds > UINT64_MAX / 1000 ? UINT64_MAX + : milliseconds * 1000)); } void From f8ed06f1c60008a9ad01a93b18af9d37859ceee6 Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Thu, 30 Jul 2026 18:15:02 +0200 Subject: [PATCH 3/3] wake watchdog with virtual time --- src/sentry_app_hang_monitor.c | 60 +++++----- src/sentry_app_hang_monitor.h | 1 - src/sentry_utils.c | 74 +++++-------- src/sentry_utils.h | 27 +++++ tests/unit/test_app_hang.c | 61 ++++++++--- tests/unit/test_utils.c | 199 ++++++++++++++++++++++++++++++++++ tests/unit/tests.inc | 1 + 7 files changed, 328 insertions(+), 95 deletions(-) diff --git a/src/sentry_app_hang_monitor.c b/src/sentry_app_hang_monitor.c index 0e169d4de2..8507e70392 100644 --- a/src/sentry_app_hang_monitor.c +++ b/src/sentry_app_hang_monitor.c @@ -60,6 +60,7 @@ static bool g_running = false; static sentry_threadid_t g_thread; static sentry_mutex_t g_wait_mutex = SENTRY__MUTEX_INIT; static sentry_cond_t g_wait_cond; +static sentry_clock_waiter_t g_waiter; static uint64_t g_timeout_ms = 0; static size_t @@ -85,30 +86,6 @@ app_hang_capture(uint64_t hang_time_ms, uint64_t tid) return true; } -void -sentry__app_hang_monitor_check(uint64_t *last_fired_heartbeat_ms) -{ - if (!sentry__app_hang_is_active() || sentry__app_hang_is_paused()) { - return; - } - - const sentry_app_hang_latch_t latch = sentry__app_hang_current_latch(); - uint64_t now = sentry__monotonic_time(); - if (!sentry__app_hang_should_capture(latch.last_heartbeat_ms, now, - g_timeout_ms, *last_fired_heartbeat_ms)) { - return; - } - - // Re-check state immediately before stackwalking to avoid reporting while - // crash handling or an explicit pause has disarmed monitoring. - if (!sentry__app_hang_is_active() || sentry__app_hang_is_paused()) { - return; - } - if (app_hang_capture(now - latch.last_heartbeat_ms, latch.target_tid)) { - *last_fired_heartbeat_ms = latch.last_heartbeat_ms; - } -} - SENTRY_THREAD_FN worker(void *arg) { @@ -120,14 +97,32 @@ worker(void *arg) sentry__mutex_unlock(&g_wait_mutex); break; } - sentry__cond_wait_timeout( - &g_wait_cond, &g_wait_mutex, SENTRY_APP_HANG_POLL_MS); + sentry__clock_waiter_wait_locked( + &g_waiter, SENTRY_APP_HANG_POLL_MS); sentry__mutex_unlock(&g_wait_mutex); if (!sentry__app_hang_is_active()) { break; } - sentry__app_hang_monitor_check(&last_fired_hb); + if (sentry__app_hang_is_paused()) { + continue; + } + + const sentry_app_hang_latch_t latch = sentry__app_hang_current_latch(); + uint64_t now = sentry__monotonic_time(); + if (!sentry__app_hang_should_capture(latch.last_heartbeat_ms, now, + g_timeout_ms, last_fired_hb)) { + continue; + } + + // Re-check state immediately before stackwalking to avoid reporting + // while crash handling or an explicit pause disarmed monitoring. + if (!sentry__app_hang_is_active() || sentry__app_hang_is_paused()) { + continue; + } + if (app_hang_capture(now - latch.last_heartbeat_ms, latch.target_tid)) { + last_fired_hb = latch.last_heartbeat_ms; + } } return 0; } @@ -141,11 +136,13 @@ sentry__app_hang_monitor_start(const sentry_options_t *options) g_timeout_ms = options->app_hang_timeout; sentry__cond_init(&g_wait_cond); + sentry__clock_waiter_init(&g_waiter, &g_wait_cond, &g_wait_mutex); // Arm before spawning: the worker uses is_active() as its run condition, so // it must already be true when the new thread first evaluates the loop. sentry__app_hang_set_active(true); if (sentry__thread_spawn(&g_thread, worker, NULL) != 0) { sentry__app_hang_set_active(false); + sentry__clock_waiter_deinit(&g_waiter); SENTRY_WARN("app-hang: failed to spawn watchdog thread"); return 1; } @@ -163,10 +160,11 @@ sentry__app_hang_monitor_stop(void) } sentry__app_hang_set_active(false); sentry__mutex_lock(&g_wait_mutex); - sentry__cond_wake(&g_wait_cond); + sentry__clock_waiter_wake_locked(&g_waiter); sentry__mutex_unlock(&g_wait_mutex); sentry__thread_join(g_thread); sentry__thread_free(&g_thread); + sentry__clock_waiter_deinit(&g_waiter); sentry__app_hang_latch_reset(); g_running = false; // g_timeout_ms are intentionally NOT cleared here: the worker @@ -192,10 +190,4 @@ sentry__app_hang_monitor_stop(void) { } -void -sentry__app_hang_monitor_check(uint64_t *last_fired_heartbeat_ms) -{ - (void)last_fired_heartbeat_ms; -} - #endif // SENTRY_HAS_THREAD_STACKWALK diff --git a/src/sentry_app_hang_monitor.h b/src/sentry_app_hang_monitor.h index 83787c4e54..c97d9fed9b 100644 --- a/src/sentry_app_hang_monitor.h +++ b/src/sentry_app_hang_monitor.h @@ -21,7 +21,6 @@ struct sentry_options_s; int sentry__app_hang_monitor_start(const struct sentry_options_s *options); void sentry__app_hang_monitor_stop(void); -void sentry__app_hang_monitor_check(uint64_t *last_fired_heartbeat_ms); // Test hook: overrides the platform thread stackwalker used by the watchdog. typedef size_t (*sentry__app_hang_stackwalk_fn)( diff --git a/src/sentry_utils.c b/src/sentry_utils.c index 5f0119eeea..fb3942bf74 100644 --- a/src/sentry_utils.c +++ b/src/sentry_utils.c @@ -20,67 +20,47 @@ #include #include -#ifdef SENTRY_UNITTEST -static volatile long g_test_clock_enabled = 0; -static uint64_t g_test_clock_monotonic_ms = 0; -static uint64_t g_test_clock_epoch_usec = 0; - -static uint64_t -add_saturate(uint64_t value, uint64_t increment) -{ - return value > UINT64_MAX - increment ? UINT64_MAX : value + increment; -} - void -sentry__test_clock_set(uint64_t monotonic_ms, uint64_t epoch_usec) +sentry__clock_waiter_init(sentry_clock_waiter_t *waiter, sentry_cond_t *cond, + sentry_mutex_t *mutex) { - sentry__atomic_store_u64(&g_test_clock_monotonic_ms, monotonic_ms); - sentry__atomic_store_u64(&g_test_clock_epoch_usec, epoch_usec); - sentry__atomic_store(&g_test_clock_enabled, 1); + waiter->mutex = mutex; + waiter->cond = cond; +#ifdef SENTRY_UNITTEST + sentry__test_clock_waiter_init(waiter); +#endif } void -sentry__test_clock_advance(uint64_t milliseconds) +sentry__clock_waiter_deinit(sentry_clock_waiter_t *waiter) { - assert(sentry__atomic_fetch(&g_test_clock_enabled)); - - uint64_t monotonic_ms - = sentry__atomic_fetch_u64(&g_test_clock_monotonic_ms); - uint64_t epoch_usec = sentry__atomic_fetch_u64(&g_test_clock_epoch_usec); - sentry__atomic_store_u64( - &g_test_clock_monotonic_ms, add_saturate(monotonic_ms, milliseconds)); - sentry__atomic_store_u64(&g_test_clock_epoch_usec, - add_saturate(epoch_usec, - milliseconds > UINT64_MAX / 1000 ? UINT64_MAX - : milliseconds * 1000)); +#ifdef SENTRY_UNITTEST + sentry__test_clock_waiter_deinit(waiter); +#else + (void)waiter; +#endif } void -sentry__test_clock_reset(void) -{ - sentry__atomic_store(&g_test_clock_enabled, 0); - sentry__atomic_store_u64(&g_test_clock_monotonic_ms, 0); - sentry__atomic_store_u64(&g_test_clock_epoch_usec, 0); -} - -bool -sentry__test_clock_is_enabled(void) +sentry__clock_waiter_wait_locked( + sentry_clock_waiter_t *waiter, uint64_t timeout_ms) { - return sentry__atomic_fetch(&g_test_clock_enabled) != 0; -} - -uint64_t -sentry__test_clock_monotonic_time(void) -{ - return sentry__atomic_fetch_u64(&g_test_clock_monotonic_ms); +#ifdef SENTRY_UNITTEST + if (sentry__test_clock_waiter_wait_locked(waiter)) { + return; + } +#endif + sentry__cond_wait_timeout(waiter->cond, waiter->mutex, timeout_ms); } -uint64_t -sentry__test_clock_usec_time(void) +void +sentry__clock_waiter_wake_locked(sentry_clock_waiter_t *waiter) { - return sentry__atomic_fetch_u64(&g_test_clock_epoch_usec); -} +#ifdef SENTRY_UNITTEST + sentry__test_clock_waiter_wake_locked(waiter); #endif + sentry__cond_wake(waiter->cond); +} #ifdef SENTRY_PLATFORM_DARWIN # include diff --git a/src/sentry_utils.h b/src/sentry_utils.h index af51a2e4b2..cf55b9372f 100644 --- a/src/sentry_utils.h +++ b/src/sentry_utils.h @@ -3,6 +3,7 @@ #include "sentry_boot.h" #include "sentry_slice.h" +#include "sentry_sync.h" #ifdef SENTRY_PLATFORM_DARWIN # include @@ -177,6 +178,32 @@ uint64_t sentry__test_clock_monotonic_time(void); uint64_t sentry__test_clock_usec_time(void); #endif +typedef struct sentry_clock_waiter_s { + sentry_mutex_t *mutex; + sentry_cond_t *cond; +#ifdef SENTRY_UNITTEST + struct sentry_clock_waiter_s *next; + uint64_t requested_revision; + uint64_t seen_revision; + uint64_t wake_revision; + bool registered; +#endif +} sentry_clock_waiter_t; + +#ifdef SENTRY_UNITTEST +void sentry__test_clock_waiter_init(sentry_clock_waiter_t *waiter); +void sentry__test_clock_waiter_deinit(sentry_clock_waiter_t *waiter); +bool sentry__test_clock_waiter_wait_locked(sentry_clock_waiter_t *waiter); +void sentry__test_clock_waiter_wake_locked(sentry_clock_waiter_t *waiter); +#endif + +void sentry__clock_waiter_init(sentry_clock_waiter_t *waiter, + sentry_cond_t *cond, sentry_mutex_t *mutex); +void sentry__clock_waiter_deinit(sentry_clock_waiter_t *waiter); +void sentry__clock_waiter_wait_locked( + sentry_clock_waiter_t *waiter, uint64_t timeout_ms); +void sentry__clock_waiter_wake_locked(sentry_clock_waiter_t *waiter); + /** * Returns the number of microseconds since the unix epoch. */ diff --git a/tests/unit/test_app_hang.c b/tests/unit/test_app_hang.c index 2286284173..2331cde803 100644 --- a/tests/unit/test_app_hang.c +++ b/tests/unit/test_app_hang.c @@ -117,6 +117,34 @@ SENTRY_TEST(app_hang_make_event) static long g_app_hang_seen; static char g_app_hang_type[32]; +static sentry_cond_t g_capture_signal; +#ifdef SENTRY__MUTEX_INIT_DYN +SENTRY__MUTEX_INIT_DYN(capture_lock) +#else +static sentry_mutex_t capture_lock = SENTRY__MUTEX_INIT; +#endif +static bool g_capture_received; + +static void +reset_capture_signal(void) +{ + SENTRY__MUTEX_INIT_DYN_ONCE(capture_lock); + sentry__cond_init(&g_capture_signal); + g_capture_received = false; +} + +static bool +wait_for_capture(void) +{ + SENTRY__MUTEX_INIT_DYN_ONCE(capture_lock); + sentry__mutex_lock(&capture_lock); + for (int i = 0; i < 10 && !g_capture_received; i++) { + sentry__cond_wait_timeout(&g_capture_signal, &capture_lock, 100); + } + const bool captured = g_capture_received; + sentry__mutex_unlock(&capture_lock); + return captured; +} static size_t fake_stackwalk(uint64_t tid, void **ips, size_t max) @@ -145,6 +173,10 @@ capture_before_send(sentry_value_t event, void *hint, void *data) strncpy(g_app_hang_type, type, sizeof(g_app_hang_type) - 1); } sentry__atomic_store(&g_app_hang_seen, 1); + sentry__mutex_lock(&capture_lock); + g_capture_received = true; + sentry__cond_wake(&g_capture_signal); + sentry__mutex_unlock(&capture_lock); sentry_value_decref(event); return sentry_value_new_null(); } @@ -157,6 +189,7 @@ SENTRY_TEST(app_hang_monitor_fires) set_test_clock(); g_app_hang_seen = 0; g_app_hang_type[0] = '\0'; + reset_capture_signal(); sentry__app_hang_latch_reset(); sentry__app_hang_monitor_set_stackwalk_fn(fake_stackwalk); @@ -169,8 +202,7 @@ SENTRY_TEST(app_hang_monitor_fires) sentry_app_hang_heartbeat(); sentry__test_clock_advance(1000); - uint64_t last_fired_heartbeat_ms = 0; - sentry__app_hang_monitor_check(&last_fired_heartbeat_ms); + TEST_CHECK(wait_for_capture()); TEST_CHECK(sentry__atomic_fetch(&g_app_hang_seen) == 1); TEST_CHECK_STRING_EQUAL(g_app_hang_type, "AppHang"); @@ -188,6 +220,7 @@ SENTRY_TEST(app_hang_pause_prevents_capture) set_test_clock(); g_app_hang_seen = 0; g_app_hang_type[0] = '\0'; + reset_capture_signal(); sentry__app_hang_latch_reset(); sentry__app_hang_monitor_set_stackwalk_fn(fake_stackwalk); @@ -202,13 +235,11 @@ SENTRY_TEST(app_hang_pause_prevents_capture) sentry_app_hang_pause(); sentry__test_clock_advance(2000); - uint64_t last_fired_heartbeat_ms = 0; - sentry__app_hang_monitor_check(&last_fired_heartbeat_ms); TEST_CHECK(sentry__atomic_fetch(&g_app_hang_seen) == 0); sentry_app_hang_heartbeat(); sentry__test_clock_advance(1000); - sentry__app_hang_monitor_check(&last_fired_heartbeat_ms); + TEST_CHECK(wait_for_capture()); TEST_CHECK(sentry__atomic_fetch(&g_app_hang_seen) == 1); TEST_CHECK_STRING_EQUAL(g_app_hang_type, "AppHang"); @@ -230,6 +261,7 @@ SENTRY_TEST(app_hang_disarm_prevents_capture) set_test_clock(); g_app_hang_seen = 0; g_app_hang_type[0] = '\0'; + reset_capture_signal(); sentry__app_hang_latch_reset(); sentry__app_hang_monitor_set_stackwalk_fn(fake_stackwalk); @@ -245,12 +277,9 @@ SENTRY_TEST(app_hang_disarm_prevents_capture) sentry__app_hang_set_active(false); sentry__test_clock_advance(2000); - uint64_t last_fired_heartbeat_ms = 0; - sentry__app_hang_monitor_check(&last_fired_heartbeat_ms); - - TEST_CHECK(sentry__atomic_fetch(&g_app_hang_seen) == 0); sentry_close(); + TEST_CHECK(sentry__atomic_fetch(&g_app_hang_seen) == 0); sentry__app_hang_monitor_set_stackwalk_fn(NULL); sentry__test_clock_reset(); } @@ -279,6 +308,10 @@ real_before_send(sentry_value_t event, void *hint, void *data) sentry_value_get_by_key(exc, "stacktrace"), "frames"); sentry__atomic_store(&g_real_frames, (long)sentry_value_get_length(frames)); sentry__atomic_store(&g_real_seen, 1); + sentry__mutex_lock(&capture_lock); + g_capture_received = true; + sentry__cond_wake(&g_capture_signal); + sentry__mutex_unlock(&capture_lock); sentry_value_decref(event); return sentry_value_new_null(); } @@ -311,6 +344,7 @@ SENTRY_TEST(app_hang_end_to_end) set_test_clock(); g_real_seen = 0; g_real_frames = 0; + reset_capture_signal(); sentry__atomic_store(&g_keep_spinning, 1); SENTRY__MUTEX_INIT_DYN_ONCE(spinner_lock); sentry__cond_init(&g_spinner_ready); @@ -334,11 +368,12 @@ SENTRY_TEST(app_hang_end_to_end) } const bool spinner_started = g_spinner_started; sentry__mutex_unlock(&spinner_lock); - TEST_ASSERT(spinner_started); + TEST_CHECK(spinner_started); - sentry__test_clock_advance(1000); - uint64_t last_fired_heartbeat_ms = 0; - sentry__app_hang_monitor_check(&last_fired_heartbeat_ms); + if (spinner_started) { + sentry__test_clock_advance(1000); + TEST_CHECK(wait_for_capture()); + } sentry__atomic_store(&g_keep_spinning, 0); sentry__thread_join(t); diff --git a/tests/unit/test_utils.c b/tests/unit/test_utils.c index 09eb4a1a71..f12f714677 100644 --- a/tests/unit/test_utils.c +++ b/tests/unit/test_utils.c @@ -7,6 +7,142 @@ #include #include +static volatile long g_test_clock_enabled = 0; +static uint64_t g_test_clock_monotonic_ms = 0; +static uint64_t g_test_clock_epoch_usec = 0; +static uint64_t g_test_clock_revision = 0; +static sentry_clock_waiter_t *g_test_clock_waiters = NULL; +#ifdef SENTRY__MUTEX_INIT_DYN +SENTRY__MUTEX_INIT_DYN(g_test_clock_waiters_lock) +#else +static sentry_mutex_t g_test_clock_waiters_lock = SENTRY__MUTEX_INIT; +#endif + +static uint64_t +add_saturate(uint64_t value, uint64_t increment) +{ + return value > UINT64_MAX - increment ? UINT64_MAX : value + increment; +} + +void +sentry__test_clock_set(uint64_t monotonic_ms, uint64_t epoch_usec) +{ + sentry__atomic_store_u64(&g_test_clock_monotonic_ms, monotonic_ms); + sentry__atomic_store_u64(&g_test_clock_epoch_usec, epoch_usec); + sentry__atomic_store(&g_test_clock_enabled, 1); +} + +void +sentry__test_clock_advance(uint64_t milliseconds) +{ + assert(sentry__atomic_fetch(&g_test_clock_enabled)); + + uint64_t monotonic_ms + = sentry__atomic_fetch_u64(&g_test_clock_monotonic_ms); + uint64_t epoch_usec = sentry__atomic_fetch_u64(&g_test_clock_epoch_usec); + sentry__atomic_store_u64( + &g_test_clock_monotonic_ms, add_saturate(monotonic_ms, milliseconds)); + sentry__atomic_store_u64(&g_test_clock_epoch_usec, + add_saturate(epoch_usec, + milliseconds > UINT64_MAX / 1000 ? UINT64_MAX + : milliseconds * 1000)); + + SENTRY__MUTEX_INIT_DYN_ONCE(g_test_clock_waiters_lock); + sentry__mutex_lock(&g_test_clock_waiters_lock); + const uint64_t revision = ++g_test_clock_revision; + for (sentry_clock_waiter_t *waiter = g_test_clock_waiters; waiter; + waiter = waiter->next) { + sentry__mutex_lock(waiter->mutex); + waiter->requested_revision = revision; + sentry__cond_wake(waiter->cond); + sentry__mutex_unlock(waiter->mutex); + } + sentry__mutex_unlock(&g_test_clock_waiters_lock); +} + +void +sentry__test_clock_reset(void) +{ + sentry__atomic_store(&g_test_clock_enabled, 0); + sentry__atomic_store_u64(&g_test_clock_monotonic_ms, 0); + sentry__atomic_store_u64(&g_test_clock_epoch_usec, 0); +} + +bool +sentry__test_clock_is_enabled(void) +{ + return sentry__atomic_fetch(&g_test_clock_enabled) != 0; +} + +uint64_t +sentry__test_clock_monotonic_time(void) +{ + return sentry__atomic_fetch_u64(&g_test_clock_monotonic_ms); +} + +uint64_t +sentry__test_clock_usec_time(void) +{ + return sentry__atomic_fetch_u64(&g_test_clock_epoch_usec); +} + +void +sentry__test_clock_waiter_init(sentry_clock_waiter_t *waiter) +{ + SENTRY__MUTEX_INIT_DYN_ONCE(g_test_clock_waiters_lock); + sentry__mutex_lock(&g_test_clock_waiters_lock); + waiter->next = g_test_clock_waiters; + waiter->requested_revision = 0; + waiter->seen_revision = 0; + waiter->wake_revision = 0; + waiter->registered = true; + g_test_clock_waiters = waiter; + sentry__mutex_unlock(&g_test_clock_waiters_lock); +} + +void +sentry__test_clock_waiter_deinit(sentry_clock_waiter_t *waiter) +{ + if (!waiter->registered) { + return; + } + SENTRY__MUTEX_INIT_DYN_ONCE(g_test_clock_waiters_lock); + sentry__mutex_lock(&g_test_clock_waiters_lock); + sentry_clock_waiter_t **next = &g_test_clock_waiters; + while (*next && *next != waiter) { + next = &(*next)->next; + } + if (*next) { + *next = waiter->next; + } + waiter->registered = false; + sentry__mutex_unlock(&g_test_clock_waiters_lock); +} + +bool +sentry__test_clock_waiter_wait_locked(sentry_clock_waiter_t *waiter) +{ + if (!sentry__test_clock_is_enabled()) { + return false; + } + + const uint64_t wake_revision = waiter->wake_revision; + while (waiter->requested_revision == waiter->seen_revision + && waiter->wake_revision == wake_revision) { + sentry__cond_wait(waiter->cond, waiter->mutex); + } + if (waiter->requested_revision > waiter->seen_revision) { + waiter->seen_revision = waiter->requested_revision; + } + return true; +} + +void +sentry__test_clock_waiter_wake_locked(sentry_clock_waiter_t *waiter) +{ + waiter->wake_revision++; +} + #ifdef SENTRY_PLATFORM_UNIX # include "sentry_unix_pageallocator.h" #endif @@ -54,6 +190,69 @@ SENTRY_TEST(virtual_clock) TEST_CHECK(!sentry__test_clock_is_enabled()); } +typedef struct { + sentry_clock_waiter_t waiter; + sentry_mutex_t mutex; + sentry_cond_t signal; + bool waiting; + bool woke; +} clock_waiter_test_t; + +SENTRY_THREAD_FN +clock_waiter_test_thread(void *data) +{ + clock_waiter_test_t *state = data; + sentry__mutex_lock(&state->mutex); + state->waiting = true; + sentry__cond_wake(&state->signal); + sentry__clock_waiter_wait_locked(&state->waiter, 1000); + state->woke = true; + sentry__cond_wake(&state->signal); + sentry__mutex_unlock(&state->mutex); + return 0; +} + +SENTRY_TEST(virtual_clock_waiter) +{ + clock_waiter_test_t state = { 0 }; + sentry__mutex_init(&state.mutex); + sentry__cond_init(&state.signal); + sentry__clock_waiter_init(&state.waiter, &state.signal, &state.mutex); + sentry_threadid_t thread; + const int spawned + = sentry__thread_spawn(&thread, clock_waiter_test_thread, &state); + TEST_CHECK_INT_EQUAL(spawned, 0); + if (spawned == 0) { + sentry__test_clock_set(1000, 1700000000000000ULL); + + sentry__mutex_lock(&state.mutex); + for (int i = 0; i < 10 && !state.waiting; i++) { + sentry__cond_wait_timeout(&state.signal, &state.mutex, 100); + } + const bool waiting = state.waiting; + sentry__mutex_unlock(&state.mutex); + TEST_CHECK(waiting); + + sentry__test_clock_advance(1); + sentry__mutex_lock(&state.mutex); + for (int i = 0; i < 10 && !state.woke; i++) { + sentry__cond_wait_timeout(&state.signal, &state.mutex, 100); + } + if (!state.woke) { + sentry__clock_waiter_wake_locked(&state.waiter); + } + const bool woke = state.woke; + sentry__mutex_unlock(&state.mutex); + + sentry__thread_join(thread); + sentry__thread_free(&thread); + TEST_CHECK(woke); + } + sentry__clock_waiter_deinit(&state.waiter); + sentry__mutex_free(&state.mutex); + sentry__test_clock_reset(); +} + static void check_url(const sentry_url_t *url) { diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 8f910bb10d..5249998011 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -416,6 +416,7 @@ XX(user_report_is_valid) XX(uuid_api) XX(uuid_v4) XX(virtual_clock) +XX(virtual_clock_waiter) XX(value_attribute) XX(value_bool) XX(value_clone_free_original_first)