diff --git a/CHANGELOG.md b/CHANGELOG.md index 98eacde70..a2ee5fa8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ - Native: store daemon logs, minidumps, crash envelopes, and scratch files in `.run` directories so they are cleaned up with the run instead of accumulating in the database root. Minidumps can still be retained with `cache_keep`, which stores `.dmp` sidecars alongside cached envelopes. ([#1976](https://github.com/getsentry/sentry-native/pull/1976)) - Linux/ARM32: prevent recursive crashes when libunwind receives an unmapped initial instruction pointer during crash handling. ([#1977](https://github.com/getsentry/sentry-native/pull/1977)) +- Destroy condition variables as approriate when no longer needed. ([#2004](https://github.com/getsentry/sentry-native/pull/2004)) ## 0.16.3 diff --git a/src/sentry_app_hang_monitor.c b/src/sentry_app_hang_monitor.c index 605402854..dc2d2b672 100644 --- a/src/sentry_app_hang_monitor.c +++ b/src/sentry_app_hang_monitor.c @@ -140,6 +140,7 @@ sentry__app_hang_monitor_start(const sentry_options_t *options) sentry__app_hang_set_active(true); if (sentry__thread_spawn(&g_thread, worker, NULL) != 0) { sentry__app_hang_set_active(false); + sentry__cond_free(&g_wait_cond); SENTRY_WARN("app-hang: failed to spawn watchdog thread"); return 1; } @@ -161,6 +162,7 @@ sentry__app_hang_monitor_stop(void) sentry__mutex_unlock(&g_wait_mutex); sentry__thread_join(g_thread); sentry__thread_free(&g_thread); + sentry__cond_free(&g_wait_cond); sentry__app_hang_latch_reset(); g_running = false; // g_timeout_ms are intentionally NOT cleared here: the worker diff --git a/src/sentry_sync.c b/src/sentry_sync.c index 4dd53680b..f489a0ba5 100644 --- a/src/sentry_sync.c +++ b/src/sentry_sync.c @@ -450,6 +450,8 @@ sentry__threadpool_free(sentry_threadpool_t *pool) sentry__thread_free(&pool->threads[i]); } sentry_free(pool->thread_name); + sentry__cond_free(&pool->work_signal); + sentry__cond_free(&pool->state_signal); sentry__mutex_free(&pool->lock); sentry_free(pool->threads); sentry_free(pool); @@ -571,6 +573,8 @@ sentry__bgworker_decref(sentry_bgworker_t *bgw) bgw->free_state(bgw->state); } sentry__thread_free(&bgw->thread_id); + sentry__cond_free(&bgw->submit_signal); + sentry__cond_free(&bgw->done_signal); sentry__mutex_free(&bgw->task_lock); sentry_free(bgw->thread_name); sentry_free(bgw); @@ -708,6 +712,7 @@ static void sentry__flush_task_decref(sentry_flush_task_t *task) { if (sentry__atomic_fetch_and_add(&task->refcount, -1) == 1) { + sentry__cond_free(&task->signal); sentry__mutex_free(&task->lock); sentry_free(task); } diff --git a/src/sentry_sync.h b/src/sentry_sync.h index 3e28a2836..01ac4478b 100644 --- a/src/sentry_sync.h +++ b/src/sentry_sync.h @@ -93,6 +93,13 @@ InitializeConditionVariable_PREVISTA( ConditionVariable->ContinueEvent = CreateEventW(NULL, FALSE, FALSE, NULL); } +inline void +DeleteConditionVariable_PREVISTA(PCONDITION_VARIABLE_PREVISTA ConditionVariable) +{ + CloseHandle(ConditionVariable->Semaphore); + CloseHandle(ConditionVariable->ContinueEvent); +} + inline BOOL SleepConditionVariableCS_PREVISTA( PCONDITION_VARIABLE_PREVISTA cv, PCRITICAL_SECTION cs, DWORD timeout) @@ -210,6 +217,8 @@ typedef struct sentry__winmutex_s sentry_mutex_t; typedef CONDITION_VARIABLE_PREVISTA sentry_cond_t; # define sentry__cond_init(CondVar) \ InitializeConditionVariable_PREVISTA(CondVar) +# define sentry__cond_free(CondVar) \ + DeleteConditionVariable_PREVISTA(CondVar) # define sentry__cond_wake WakeConditionVariable_PREVISTA # define sentry__cond_wake_all WakeAllConditionVariable_PREVISTA # define sentry__cond_wait_timeout(CondVar, Lock, Timeout) \ @@ -218,6 +227,7 @@ typedef CONDITION_VARIABLE_PREVISTA sentry_cond_t; # else typedef CONDITION_VARIABLE sentry_cond_t; # define sentry__cond_init(CondVar) InitializeConditionVariable(CondVar) +# define sentry__cond_free(CondVar) ((void)(CondVar)) # define sentry__cond_wake WakeConditionVariable # define sentry__cond_wake_all WakeAllConditionVariable # define sentry__cond_wait_timeout(CondVar, Lock, Timeout) \ @@ -355,6 +365,7 @@ typedef pthread_cond_t sentry_cond_t; sentry_cond_t tmp = PTHREAD_COND_INITIALIZER; \ *(CondVar) = tmp; \ } while (0) +# define sentry__cond_free(CondVar) pthread_cond_destroy(CondVar) # define sentry__cond_wait(Cond, Mutex) \ do { \ if (sentry__block_for_signal_handler()) { \ diff --git a/tests/unit/test_sync.c b/tests/unit/test_sync.c index d73df5106..8453076f0 100644 --- a/tests/unit/test_sync.c +++ b/tests/unit/test_sync.c @@ -68,8 +68,8 @@ trailing_task(void *data, void *UNUSED(state)) sentry__mutex_lock(&executed_lock); bool *executed = (bool *)data; *executed = true; - sentry__mutex_unlock(&executed_lock); sentry__cond_wake(&trailing_task_done); + sentry__mutex_unlock(&executed_lock); } static bool @@ -140,6 +140,8 @@ SENTRY_TEST(task_queue) sentry__mutex_lock(&executed_lock); sentry__cond_wait_timeout(&trailing_task_done, &executed_lock, 1000); TEST_CHECK(executed_after_shutdown); + sentry__cond_free(&trailing_task_done); + sentry__mutex_unlock(&executed_lock); } SENTRY_TEST(bgworker_flush) @@ -361,6 +363,7 @@ SENTRY_TEST(bgworker_delayed_priority) TEST_CHECK_INT_EQUAL(os.order[1], 2); // immediate (submitted later) sentry__bgworker_decref(bgw); + sentry__cond_free(&blocker_signal); } static void @@ -411,6 +414,7 @@ SENTRY_TEST(bgworker_delayed_current) TEST_CHECK_INT_EQUAL(os.order[1], 2); sentry__bgworker_decref(bgw); + sentry__cond_free(&blocker_signal); } SENTRY_TEST(bgworker_delayed_head) @@ -481,6 +485,7 @@ SENTRY_TEST(bgworker_delayed_drop_current) TEST_CHECK_INT_EQUAL(os.order[2], 2); sentry__bgworker_decref(bgw); + sentry__cond_free(&blocker_signal); } SENTRY_TEST(bgworker_delayed_drop_next) @@ -523,6 +528,7 @@ SENTRY_TEST(bgworker_delayed_drop_next) TEST_CHECK_INT_EQUAL(os.order[2], 3); sentry__bgworker_decref(bgw); + sentry__cond_free(&blocker_signal); } SENTRY_TEST(bgworker_delayed_cleanup) @@ -735,10 +741,8 @@ SENTRY_TEST(threadpool_flush_wakes_all) sentry__threadpool_free(pool); TEST_CHECK(woke_all); -#ifndef SENTRY_PLATFORM_WINDOWS - pthread_cond_destroy(&flush_state.cond); - pthread_cond_destroy(&task_state.cond); -#endif + sentry__cond_free(&flush_state.cond); + sentry__cond_free(&task_state.cond); sentry__mutex_free(&flush_state.lock); sentry__mutex_free(&task_state.lock); } @@ -772,9 +776,7 @@ SENTRY_TEST(threadpool_ordered_parallel) sentry__threadpool_shutdown(pool); sentry__threadpool_free(pool); -#ifndef SENTRY_PLATFORM_WINDOWS - pthread_cond_destroy(&state.cond); -#endif + sentry__cond_free(&state.cond); sentry__mutex_free(&state.lock); } @@ -943,9 +945,7 @@ SENTRY_TEST(threadpool_max_pending) sentry__threadpool_shutdown(pool); sentry__threadpool_free(pool); -#ifndef SENTRY_PLATFORM_WINDOWS - pthread_cond_destroy(&state.cond); -#endif + sentry__cond_free(&state.cond); sentry__mutex_free(&state.lock); } @@ -1056,9 +1056,7 @@ SENTRY_TEST(threadpool_commit_reentry) sentry__threadpool_shutdown(pool); sentry__threadpool_free(pool); -#ifndef SENTRY_PLATFORM_WINDOWS - pthread_cond_destroy(&state.cond); -#endif + sentry__cond_free(&state.cond); sentry__mutex_free(&state.lock); } @@ -1327,10 +1325,8 @@ SENTRY_TEST(cond_wake_all) TEST_CHECK_INT_EQUAL( sentry__atomic_fetch(&state.woke), COND_WAKE_ALL_THREADS); -#ifndef SENTRY_PLATFORM_WINDOWS - pthread_cond_destroy(&state.ready_cond); - pthread_cond_destroy(&state.waiting_cond); -#endif + sentry__cond_free(&state.ready_cond); + sentry__cond_free(&state.waiting_cond); sentry__mutex_free(&state.mutex); } @@ -1353,7 +1349,7 @@ SENTRY_TEST(cond_wait_timeout_overflow) sentry__mutex_unlock(&mutex); sentry__mutex_free(&mutex); - pthread_cond_destroy(&cond); + sentry__cond_free(&cond); #endif }