Skip to content

Commit f4777bb

Browse files
fix(parser): remove ICP __del__ loguru cleanup that deadlocked pytest exit
InteractiveContextParser.__del__ called loguru.remove() during GC of multiple parsers, re-entering loguru's handler lock and hanging after all tests passed. Replace with an explicit close() and restore spawn for ProcessPoolExecutor in language derivation. Co-authored-by: Arash Ashrafzadeh <arashmath16@gmail.com>
1 parent aa7681f commit f4777bb

3 files changed

Lines changed: 37 additions & 13 deletions

File tree

src/dylan/parser/interactive_context_parser.py

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -144,14 +144,27 @@ def _init_icp_log_settings(
144144
self._log = loguru_logger.bind(icp_id=self._icp_id)
145145
sync_dylan_stdlib_level_for_icp(log_level)
146146

147-
def _configure_icp_sinks(self) -> None:
148-
"""Register loguru sinks filtered to this parser's ``icp_id`` (or remove prior registrations)."""
147+
def _remove_icp_log_sinks(self) -> None:
148+
"""Remove loguru sinks registered for this parser's ``icp_id``."""
149149
for hid in self._icp_log_handler_ids:
150150
try:
151151
loguru_logger.remove(hid)
152152
except ValueError:
153153
pass
154154
self._icp_log_handler_ids.clear()
155+
156+
def close(self) -> None:
157+
"""Remove per-parser loguru sinks.
158+
159+
Call when a parser instance is no longer needed. Do not rely on ``__del__`` for cleanup:
160+
removing loguru handlers during interpreter shutdown can deadlock on loguru's internal lock
161+
when several parsers are collected together (e.g. after pytest).
162+
"""
163+
self._remove_icp_log_sinks()
164+
165+
def _configure_icp_sinks(self) -> None:
166+
"""Register loguru sinks filtered to this parser's ``icp_id`` (or remove prior registrations)."""
167+
self._remove_icp_log_sinks()
155168
if self._log_level == "off":
156169
return
157170
lu_level = "ERROR" if self._log_level == "error" else "WARNING"
@@ -183,17 +196,6 @@ def icp_only(record: dict) -> bool:
183196
)
184197
self._icp_log_handler_ids.append(hid)
185198

186-
def __del__(self) -> None:
187-
"""Best-effort removal of this parser's loguru sinks."""
188-
try:
189-
for hid in list(getattr(self, "_icp_log_handler_ids", ())):
190-
try:
191-
loguru_logger.remove(hid)
192-
except ValueError:
193-
pass
194-
except Exception:
195-
pass
196-
197199
def _log_nonoptional_adjust_limit_exceeded(self) -> None:
198200
"""Emit error when the non-optional adjustment loop exceeds its pass bound."""
199201
self._log.error(

src/dylan/parser/language_derivation.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import itertools
66
import logging
7+
import multiprocessing as mp
78
import os
89
import random
910
from collections import defaultdict
@@ -57,6 +58,15 @@ def _layered_run_status(renderable: str) -> AbstractContextManager[Any]:
5758
_LAYERED_WORKER_PARSER: Any = None
5859

5960

61+
def _language_derivation_mp_context() -> mp.context.BaseContext:
62+
"""Return a ``spawn`` multiprocessing context for language-derivation worker pools.
63+
64+
Avoids ``fork()`` from a multi-threaded parent (e.g. pytest on Linux), which can deadlock
65+
worker processes and trigger deprecation warnings.
66+
"""
67+
return mp.get_context("spawn")
68+
69+
6070
@dataclass(frozen=True)
6171
class LanguageDerivationRecord:
6272
"""Structured outcome from one candidate sentence in language derivation."""
@@ -685,6 +695,7 @@ def emit_completion_success_only(layer_idx: int, record: LanguageDerivationRecor
685695
if workers > 1:
686696
with ProcessPoolExecutor(
687697
max_workers=workers,
698+
mp_context=_language_derivation_mp_context(),
688699
initializer=_init_layered_language_worker,
689700
initargs=(
690701
str(Path(grammar_path)),
@@ -1286,6 +1297,7 @@ def run(
12861297
tasks_iter = ((words, speaker, addressee) for words in candidates)
12871298
with ProcessPoolExecutor(
12881299
max_workers=workers,
1300+
mp_context=_language_derivation_mp_context(),
12891301
initializer=_init_language_worker,
12901302
initargs=(
12911303
str(Path(grammar_path)),

tests/test_icp_logging.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,16 @@ def test_log_level_warning_suppresses_info() -> None:
6363
assert not any("Parsed" in msg for _, msg in captured)
6464

6565

66+
def test_icp_logging_suite_exits_after_multiple_terminal_parsers() -> None:
67+
"""Regression: two terminal-log parsers must not deadlock loguru sink removal at GC."""
68+
parsers = [
69+
InteractiveContextParser(FIXTURE, log_level="error", log_output="terminal"),
70+
InteractiveContextParser(FIXTURE, log_level="warning", log_output="terminal"),
71+
]
72+
for parser in parsers:
73+
parser.close()
74+
75+
6676
def test_log_output_file_writes_under_log_dir(tmp_path: Path) -> None:
6777
"""``log_output=file`` with *log_dir* creates a log file and does not require a terminal sink."""
6878
log_dir = tmp_path / "logs"

0 commit comments

Comments
 (0)