Skip to content

Commit b5073b2

Browse files
committed
Revert the changes to the span batcher - this needs to be a separate class from batcher but that will not be done in this set of changes. Update batcher deadlock fix to use weakmethod, to be consistent with the approach taken in monitor.py
1 parent 78b0f09 commit b5073b2

2 files changed

Lines changed: 45 additions & 16 deletions

File tree

sentry_sdk/_batcher.py

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -38,24 +38,23 @@ def __init__(
3838

3939
self._flusher: "Optional[threading.Thread]" = None
4040
self._flusher_pid: "Optional[int]" = None
41+
self._reset_thread_state()
4142

42-
self_ref = weakref.ref(self)
43+
# See https://github.com/getsentry/sentry-python/blob/051cc01640a29bfd64b1f1e2e3414c02f027dd1b/sentry_sdk/monitor.py#L41-L50
44+
if hasattr(os, "register_at_fork"):
45+
weak_reset = weakref.WeakMethod(self._reset_thread_state)
4346

44-
def _reset_thread_state() -> None:
45-
batcher = self_ref()
47+
def _reset_in_child() -> None:
48+
method = weak_reset()
49+
if method is not None:
50+
method()
4651

47-
if batcher is not None:
48-
batcher._flusher = None
49-
batcher._lock = threading.Lock()
50-
batcher._flusher_pid = None
52+
os.register_at_fork(after_in_child=_reset_in_child)
5153

52-
# Same as https://github.com/getsentry/sentry-python/issues/6148.
53-
# If os.fork() runs while another thread holds self._lock,
54-
# the child inherits the lock locked but the holding thread does
55-
# not exist in the child, so the lock can never be released and
56-
# _ensure_thread deadlocks forever.
57-
if hasattr(os, "register_at_fork"):
58-
os.register_at_fork(after_in_child=_reset_thread_state)
54+
def _reset_thread_state(self) -> None:
55+
self._flusher = None
56+
self._lock = threading.Lock()
57+
self._flusher_pid = None
5958

6059
def _ensure_thread(self) -> bool:
6160
"""For forking processes we might need to restart this thread.

sentry_sdk/_span_batcher.py

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
from collections import defaultdict
22
from datetime import datetime, timezone
3-
from typing import TYPE_CHECKING
3+
import os
4+
import threading
5+
from typing import TYPE_CHECKING, Optional
6+
import weakref
47

58
from sentry_sdk._batcher import Batcher
69
from sentry_sdk.envelope import Envelope, Item, PayloadRef
@@ -31,14 +34,41 @@ def __init__(
3134
capture_func: "Callable[[Envelope], None]",
3235
record_lost_func: "Callable[..., None]",
3336
) -> None:
34-
super().__init__(capture_func, record_lost_func)
3537
# Spans from different traces cannot be emitted in the same envelope
3638
# since the envelope contains a shared trace header. That's why we bucket
3739
# by trace_id, so that we can then send the buckets each in its own
3840
# envelope.
3941
# trace_id -> span buffer
4042
self._span_buffer: dict[str, list["StreamedSpan"]] = defaultdict(list)
4143
self._running_size: dict[str, int] = defaultdict(lambda: 0)
44+
self._capture_func = capture_func
45+
self._record_lost_func = record_lost_func
46+
self._running = True
47+
self._lock = threading.Lock()
48+
self._active: "threading.local" = threading.local()
49+
50+
self._flush_event: "threading.Event" = threading.Event()
51+
52+
self._flusher: "Optional[threading.Thread]" = None
53+
self._flusher_pid: "Optional[int]" = None
54+
55+
self._reset_thread_state()
56+
57+
# See https://github.com/getsentry/sentry-python/blob/051cc01640a29bfd64b1f1e2e3414c02f027dd1b/sentry_sdk/monitor.py#L41-L50
58+
if hasattr(os, "register_at_fork"):
59+
weak_reset = weakref.WeakMethod(self._reset_thread_state)
60+
61+
def _reset_in_child() -> None:
62+
method = weak_reset()
63+
if method is not None:
64+
method()
65+
66+
os.register_at_fork(after_in_child=_reset_in_child)
67+
68+
def _reset_thread_state(self) -> None:
69+
self._flusher = None
70+
self._lock = threading.Lock()
71+
self._flusher_pid = None
4272

4373
def add(self, span: "StreamedSpan") -> None:
4474
# Bail out if the current thread is already executing batcher code.

0 commit comments

Comments
 (0)