From c4957353483c1d2b86b1dad3fed90c2f6a0bb034 Mon Sep 17 00:00:00 2001 From: Buck Evan Date: Tue, 8 Jul 2025 17:24:56 -0500 Subject: [PATCH 1/2] fix(sessions): clean up threading on kill+flush This follows the precedent set in transport.flush, and helps me with my project to assert thread hygeine in sentry test suite (related: DI-1008), but also makes the system much more deterministic for everyone. This will cause shutdown to be quite slow until #4561 goes in. So I'd reject this if you reject that. --- sentry_sdk/sessions.py | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/sessions.py b/sentry_sdk/sessions.py index a5dd589ee9..50afadd301 100644 --- a/sentry_sdk/sessions.py +++ b/sentry_sdk/sessions.py @@ -165,8 +165,9 @@ def __init__( def flush(self): # type: (...) -> None - pending_sessions = self.pending_sessions - self.pending_sessions = [] + with self._thread_lock: + pending_sessions = self.pending_sessions + self.pending_sessions = [] with self._aggregate_lock: pending_aggregates = self.pending_aggregates @@ -190,6 +191,26 @@ def flush(self): if len(envelope.items) > 0: self.capture_func(envelope) + # hygiene: deterministically clean up any stopping thread + if not self._thread_stopping(): + return + with self._thread_lock: + if not self._thread_stopping(): + return + if self._thread: # typing + self._thread.join() + self._thread = None + self._thread_for_pid = None + + def _thread_stopping(self): + # type: (...) -> bool + return ( + not self._running + and self._thread is not None + # we are the parent thread: + and self._thread_for_pid == os.getpid() + ) + def _ensure_running(self): # type: (...) -> None """ From 8d24e889d69a41545a5a1237b3b3264b2fd4cc8e Mon Sep 17 00:00:00 2001 From: Buck Evan Date: Wed, 9 Jul 2025 13:18:44 -0500 Subject: [PATCH 2/2] fix after rebase --- sentry_sdk/sessions.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/sentry_sdk/sessions.py b/sentry_sdk/sessions.py index 50afadd301..2fc4abb407 100644 --- a/sentry_sdk/sessions.py +++ b/sentry_sdk/sessions.py @@ -192,20 +192,20 @@ def flush(self): self.capture_func(envelope) # hygiene: deterministically clean up any stopping thread - if not self._thread_stopping(): + if not self._should_join_thread(): return with self._thread_lock: - if not self._thread_stopping(): + if not self._should_join_thread(): return if self._thread: # typing self._thread.join() self._thread = None self._thread_for_pid = None - def _thread_stopping(self): + def _should_join_thread(self): # type: (...) -> bool return ( - not self._running + not self.__shutdown_requested.is_set() and self._thread is not None # we are the parent thread: and self._thread_for_pid == os.getpid() @@ -217,8 +217,8 @@ def _ensure_running(self): Check that we have an active thread to run in, or create one if not. Note that this might fail (e.g. in Python 3.12 it's not possible to - spawn new threads at interpreter shutdown). In that case self._running - will be False after running this function. + spawn new threads at interpreter shutdown). In that case + `__shutdown_requested` will be set after running this function. """ if self._thread_for_pid == os.getpid() and self._thread is not None: return None