Skip to content

Commit fa02d40

Browse files
winedarkseaclaude
andcommitted
fix: attach ring buffers in the parent for fork producers
`Producer.__call__` attached the five ring-buffer shared-memory segments from inside the child for every start method. Under `fork` that is not safe: `SharedMemory(name=..., create=False)` calls `multiprocessing.resource_tracker.register`, which takes a module-level `threading.RLock` that CPython does not reinitialize after `fork`. A child that inherits that lock held by a thread which does not exist in the child blocks on the attach forever -- no timeout, nothing to interrupt it. `WorkerBase.__init__` already handles this: it sets `_lazy_init = start_method != "fork"` and, for `fork`, initializes logging and attaches the rings in the parent so the child inherits the mappings. `Producer` never got the same treatment. It does now, mirroring the worker exactly. Also logs the attach on both sides, as the worker already does, so a child that does wedge there says so instead of going silent between two unrelated lines. Verified on Linux (Python 3.12.13, TensorFlow 2.21.0): - The new test counts shared-memory attaches performed outside the process that set the session up. On `main` a fork producer child performs five; with this change, none. It fails on `main` and passes here, deterministically -- the deadlock is a race, so the invariant is what gets asserted, not the hang. - `fork` lane: 7 passed, 1 skipped. - `inference` + `inference_pipeline` + `v2_4` + `core_py`, non-fork: 352 passed. - 18 full `fork`-lane runs on this machine (10 unpinned, 8 pinned to 2 cores) never reproduced the CI hang, so this is not shown to change its rate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent d8b52c3 commit fa02d40

3 files changed

Lines changed: 96 additions & 2 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2121

2222
### Bugfixes
2323

24+
- Producers no longer attach the ring buffers from inside a `fork` child. `SharedMemory(create=False)` calls `multiprocessing.resource_tracker.register`, whose module-level lock CPython does not reinitialize after `fork`, so a child that inherits it held by a thread which does not exist in the child blocks on the attach forever. The workers already avoided this by attaching in the parent and inheriting the mappings (`WorkerBase.__init__`); producers now follow the same rule. Measured on Linux: five attaches per producer child before, none after. This is a candidate cause of the intermittent 600 s hangs in the `fork` CI lane, where a wedged producer child logs its logging init and never reaches "Waiting for start signal" -- exactly the window the attaches sit in.
2425
- Fixed corrupt rows in acoustic prediction and encoding output: growing the internal result buffer via `numpy.ndarray.resize` (together with an off-by-one in the initial segment count) could leave stale or uninitialized values in some segments. Buffers are now reallocated and copied, so all output rows are correct (#50).
2526
- Geo model v3.0 caches now self-heal across releases: previously a cached ProtoBuf SavedModel or generated label files from an older release were never detected as stale, so a geo model version bump could keep serving outdated species labels/counts. The SavedModel now records its source release and the label files are validated against the current labels, so both are re-fetched/regenerated when they no longer match (#41).
2627

src/birdnet/acoustic/inference/core/producer.py

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,21 @@ def __init__(
154154

155155
self._cancel_event = cancel_event
156156

157+
# Same rule the workers follow (see WorkerBase.__init__): only "fork"
158+
# children inherit parent state, so only then may logging and the ring
159+
# buffers be set up here in the parent. Attaching them in the child instead
160+
# is not fork-safe -- SharedMemory(create=False) calls
161+
# resource_tracker.register(), which takes a module-level threading lock
162+
# that CPython does not reinitialize after fork. A child that inherits it
163+
# held by a thread which no longer exists blocks on the attach forever.
164+
self._lazy_init = start_method != "fork"
165+
166+
if not self._lazy_init:
167+
self._init_logging()
168+
self._load_ring_buffers()
169+
157170
def _load_ring_buffers(self) -> None:
171+
self._log("Attaching ring buffers...")
158172
self._shm_file_indices, self._ring_file_indices = (
159173
self._rf_file_indices.attach_and_get_array()
160174
)
@@ -168,6 +182,7 @@ def _load_ring_buffers(self) -> None:
168182
self._rf_batch_sizes.attach_and_get_array()
169183
)
170184
self._shm_ring_flags, self._ring_flags = self._rf_flags.attach_and_get_array()
185+
self._log("Attached ring buffers.")
171186

172187
def get_segments_from_input(
173188
self, input_idx: int, inp_data: Path | tuple[np.ndarray, int]
@@ -541,10 +556,12 @@ def _log(self, message: str) -> None:
541556
self._logger.debug(f"P_{os.getpid()}: {message}")
542557

543558
def __call__(self) -> None:
544-
self._init_logging()
559+
if self._lazy_init:
560+
self._init_logging()
545561

546562
try:
547-
self._load_ring_buffers()
563+
if self._lazy_init:
564+
self._load_ring_buffers()
548565

549566
self._run_main_loop()
550567
except Exception as e:
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
"""Under ``fork``, no pipeline child may attach the ring buffers itself.
2+
3+
``SharedMemory(name=..., create=False)`` calls
4+
``multiprocessing.resource_tracker.register``, which takes a module-level
5+
``threading.RLock`` that CPython does not reinitialize after ``fork``. A child
6+
that inherits that lock held by a thread which does not exist in the child
7+
blocks on the attach forever, with no timeout and nothing to interrupt it.
8+
9+
``fork`` children inherit the parent's mappings, so they never need to attach:
10+
the parent does it before forking and the child uses what it inherited. This
11+
pins that invariant by counting the attaches that happen in a process other than
12+
the one that set the session up. It is deterministic -- the deadlock itself is a
13+
race, so it is not what gets asserted here.
14+
"""
15+
16+
import multiprocessing as mp
17+
import os
18+
from multiprocessing import resource_tracker
19+
from pathlib import Path
20+
21+
import numpy as np
22+
import pytest
23+
24+
from birdnet.model_loader import load
25+
from birdnet_tests.helper import use_fork_or_skip
26+
27+
SAMPLE_RATE = 48_000
28+
CLIP_DURATION_S = 9.0
29+
30+
_original_register = resource_tracker.register
31+
_parent_pid: int | None = None
32+
_registrations_path: Path | None = None
33+
34+
35+
def _tracing_register(name: str, rtype: str) -> None:
36+
# Runs in the parent and in every fork child. Only the child copies matter,
37+
# and a child cannot report back through memory, so they append to a file.
38+
if os.getpid() != _parent_pid and rtype == "shared_memory":
39+
assert _registrations_path is not None
40+
with _registrations_path.open("a", encoding="utf-8") as f:
41+
f.write(f"{os.getpid()} {name}\n")
42+
_original_register(name, rtype)
43+
44+
45+
@pytest.mark.fork
46+
def test_fork_children_do_not_attach_shared_memory_themselves(tmp_path: Path) -> None:
47+
use_fork_or_skip()
48+
assert mp.get_start_method(allow_none=True) == "fork"
49+
50+
global _parent_pid, _registrations_path
51+
_parent_pid = os.getpid()
52+
_registrations_path = tmp_path / "shm_registrations.txt"
53+
54+
rng = np.random.default_rng(0)
55+
audio = rng.standard_normal(int(SAMPLE_RATE * CLIP_DURATION_S)).astype(np.float32)
56+
57+
resource_tracker.register = _tracing_register # type: ignore[assignment]
58+
try:
59+
model = load("acoustic", "2.4", "tf", library="tflite")
60+
with model.predict_session(n_workers=1, top_k=None) as session:
61+
session.run_arrays((audio * 0.1, SAMPLE_RATE))
62+
finally:
63+
resource_tracker.register = _original_register # type: ignore[assignment]
64+
65+
registrations = (
66+
_registrations_path.read_text(encoding="utf-8").splitlines()
67+
if _registrations_path.exists()
68+
else []
69+
)
70+
71+
assert not registrations, (
72+
f"{len(registrations)} shared-memory attach(es) happened in a fork child "
73+
f"instead of being inherited from the parent. Each one calls "
74+
f"resource_tracker.register, whose lock is not reinitialized after fork and "
75+
f"can deadlock the child permanently: {registrations}"
76+
)

0 commit comments

Comments
 (0)