diff --git a/CHANGELOG.md b/CHANGELOG.md index 488c3f9a5..f2cdb42c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,12 @@ ## Unreleased +**Features**: + +- Added `sentry_app_hang_pause` to allow pausing of the app hang detection mechanism. This is useful in settings where an app can be put in the background, causing the monitored thread to suspend execution. ([#1928](https://github.com/getsentry/sentry-native/pull/1928)) + **Fixes**: + - Honor checks before launching crash reporter ([#1906](https://github.com/getsentry/sentry-native/pull/1906)) ## 0.16.0 diff --git a/include/sentry.h b/include/sentry.h index daac01a58..f86190147 100644 --- a/include/sentry.h +++ b/include/sentry.h @@ -2829,6 +2829,18 @@ SENTRY_EXPERIMENTAL_API uint64_t sentry_options_get_app_hang_timeout( */ SENTRY_EXPERIMENTAL_API void sentry_app_hang_heartbeat(void); +/** + * Pauses app-hang detection without changing the watched thread. + * + * While paused, the watchdog does not capture app hangs. A subsequent heartbeat + * from the watched thread automatically resumes detection. Heartbeats from + * other threads continue to be ignored. + * + * This function is a no-op unless app-hang detection is enabled via + * `sentry_options_set_enable_app_hang_tracking`. + */ +SENTRY_EXPERIMENTAL_API void sentry_app_hang_pause(void); + /** * Type of the `before_send_metric` callback. * diff --git a/src/sentry_app_hang_latch.c b/src/sentry_app_hang_latch.c index 334e89f4f..75d8dd8b6 100644 --- a/src/sentry_app_hang_latch.c +++ b/src/sentry_app_hang_latch.c @@ -54,6 +54,7 @@ sentry__app_hang_should_capture( static uint64_t g_target_tid = 0; static uint64_t g_last_heartbeat_ms = 0; static volatile long g_app_hang_active = 0; +static volatile long g_app_hang_paused = 0; void sentry__app_hang_set_active(bool active) @@ -67,6 +68,18 @@ sentry__app_hang_is_active(void) return sentry__atomic_fetch(&g_app_hang_active) != 0; } +void +sentry__app_hang_set_paused(bool paused) +{ + sentry__atomic_store(&g_app_hang_paused, paused ? 1 : 0); +} + +bool +sentry__app_hang_is_paused(void) +{ + return sentry__atomic_fetch(&g_app_hang_paused) != 0; +} + uint64_t sentry__app_hang_current_tid(void) { @@ -97,6 +110,7 @@ sentry__app_hang_latch_reset(void) { sentry__atomic_store_u64(&g_target_tid, 0); sentry__atomic_store_u64(&g_last_heartbeat_ms, 0); + sentry__atomic_store(&g_app_hang_paused, 0); } void @@ -117,5 +131,17 @@ sentry_app_hang_heartbeat(void) // ignore heartbeats from other threads sentry__atomic_store_u64( &g_last_heartbeat_ms, sentry__monotonic_time()); + // Publish the fresh timestamp before resuming so the watchdog cannot + // observe an unpaused detector paired with a stale heartbeat. + sentry__atomic_store(&g_app_hang_paused, 0); + } +} + +void +sentry_app_hang_pause(void) +{ + if (!sentry__app_hang_is_active()) { + return; } + sentry__app_hang_set_paused(true); } diff --git a/src/sentry_app_hang_latch.h b/src/sentry_app_hang_latch.h index 5d0123543..a3042327e 100644 --- a/src/sentry_app_hang_latch.h +++ b/src/sentry_app_hang_latch.h @@ -26,6 +26,9 @@ void sentry__app_hang_set_active(bool active); // Whether app-hang detection is currently armed. bool sentry__app_hang_is_active(void); +void sentry__app_hang_set_paused(bool paused); +bool sentry__app_hang_is_paused(void); + sentry_value_t sentry__app_hang_make_event( void **ips, size_t frame_count, uint64_t freeze_ms); diff --git a/src/sentry_app_hang_monitor.c b/src/sentry_app_hang_monitor.c index ef5f2d9cd..605402854 100644 --- a/src/sentry_app_hang_monitor.c +++ b/src/sentry_app_hang_monitor.c @@ -99,6 +99,9 @@ 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(); @@ -108,6 +111,9 @@ worker(void *arg) 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. diff --git a/src/sentry_app_hang_monitor.h b/src/sentry_app_hang_monitor.h index c97d9fed9..9ecad56f8 100644 --- a/src/sentry_app_hang_monitor.h +++ b/src/sentry_app_hang_monitor.h @@ -9,7 +9,11 @@ struct sentry_options_s; // Interval at which the watchdog samples the heartbeat. -#define SENTRY_APP_HANG_POLL_MS 500 +#if defined(SENTRY_UNITTEST) +# define SENTRY_APP_HANG_POLL_MS 10 +#else +# define SENTRY_APP_HANG_POLL_MS 500 +#endif // Smallest timeout the watchdog can resolve meaningfully. A genuine hang fires // somewhere in [timeout, timeout + POLL_MS) depending on the phase between the diff --git a/tests/unit/test_app_hang.c b/tests/unit/test_app_hang.c index 25e85caf6..176e8eee5 100644 --- a/tests/unit/test_app_hang.c +++ b/tests/unit/test_app_hang.c @@ -43,6 +43,38 @@ SENTRY_TEST(app_hang_latch) sentry__app_hang_set_active(false); } +SENTRY_TEST(app_hang_pause_resumes_on_heartbeat) +{ +#if !SENTRY_HAS_THREAD_STACKWALK + SKIP_TEST(); +#endif + sentry__app_hang_latch_reset(); + sentry__app_hang_set_active(true); + sentry_app_hang_heartbeat(); + + sentry_app_hang_latch_t l = sentry__app_hang_current_latch(); + uint64_t target = l.target_tid; + uint64_t first_heartbeat = l.last_heartbeat_ms; + TEST_CHECK(target != 0); + TEST_CHECK(first_heartbeat != 0); + + sentry_app_hang_pause(); + l = sentry__app_hang_current_latch(); + TEST_CHECK(sentry__app_hang_is_paused()); + TEST_CHECK(l.target_tid == target); + TEST_CHECK(l.last_heartbeat_ms == first_heartbeat); + + sleep_ms(1); + sentry_app_hang_heartbeat(); + l = sentry__app_hang_current_latch(); + TEST_CHECK(!sentry__app_hang_is_paused()); + TEST_CHECK(l.target_tid == target); + TEST_CHECK(l.last_heartbeat_ms > first_heartbeat); + + sentry__app_hang_latch_reset(); + sentry__app_hang_set_active(false); +} + SENTRY_TEST(app_hang_make_event) { void *ips[2] = { (void *)0x1000, (void *)0x2000 }; @@ -138,6 +170,43 @@ SENTRY_TEST(app_hang_monitor_fires) sentry__app_hang_monitor_set_stackwalk_fn(NULL); } +SENTRY_TEST(app_hang_pause_prevents_capture) +{ +#if !SENTRY_HAS_THREAD_STACKWALK + SKIP_TEST(); +#endif + g_app_hang_seen = 0; + g_app_hang_type[0] = '\0'; + sentry__app_hang_latch_reset(); + sentry__app_hang_monitor_set_stackwalk_fn(fake_stackwalk); + + SENTRY_TEST_OPTIONS_NEW(options); + 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_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); + 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); + } + + 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(app_hang_disarm_prevents_capture) { // Mirrors app_hang_monitor_fires, but disarms after latching. This is the diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index 6789e1d92..b88e07453 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -3,6 +3,8 @@ XX(app_hang_end_to_end) XX(app_hang_latch) XX(app_hang_make_event) XX(app_hang_monitor_fires) +XX(app_hang_pause_prevents_capture) +XX(app_hang_pause_resumes_on_heartbeat) XX(app_hang_should_capture) XX(assert_sdk_name) XX(assert_sdk_user_agent)