Skip to content

Commit 47584e2

Browse files
Kowserclaude
andcommitted
feat(automator): add worker_isolation module (no callers yet)
CONDUCTOR_WORKER_ISOLATION env knob (process default / thread), the Process->Thread shim moved verbatim from worker_manager plus the missing Queue->queue.Queue half, thread-safe signal.signal, idempotency flag. Dead code at this commit — wired up in the next one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent bf55db9 commit 47584e2

2 files changed

Lines changed: 279 additions & 0 deletions

File tree

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
# Copyright (c) 2025 Agentspan
2+
# Licensed under the MIT License. See LICENSE file in the project root for details.
3+
4+
"""Worker isolation mode — ``process`` (default) or ``thread``.
5+
6+
``CONDUCTOR_WORKER_ISOLATION=thread`` replaces :class:`TaskHandler`'s
7+
multiprocessing primitives (``Process``, ``Queue``) with thread equivalents.
8+
For environments where multiprocessing's fork+exec bootstraps (spawn
9+
children, ``resource_tracker``) fail — e.g. minimal microVM guests. Threads
10+
share the parent's memory, so the log relay needs no IPC and worker payloads
11+
need no pickling.
12+
13+
The default (``process``, or the variable unset) leaves every existing code
14+
path untouched: ``multiprocessing.Process`` under the ``spawn`` start method.
15+
16+
Tradeoffs (thread mode only): no per-worker force-kill (``terminate``/``kill``
17+
are no-ops; shutdown is cooperative), CPU-bound workers share the GIL, and
18+
``signal.signal`` becomes a no-op off the main thread instead of raising
19+
``ValueError``.
20+
"""
21+
22+
from __future__ import annotations
23+
24+
import logging
25+
import os
26+
import queue
27+
import threading
28+
from typing import Any
29+
30+
logger = logging.getLogger(__name__)
31+
32+
WORKER_ISOLATION_ENV = "CONDUCTOR_WORKER_ISOLATION"
33+
ISOLATION_PROCESS = "process"
34+
ISOLATION_THREAD = "thread"
35+
_VALID_MODES = frozenset({ISOLATION_PROCESS, ISOLATION_THREAD})
36+
37+
_invalid_mode_warned = False
38+
39+
40+
def isolation_mode() -> str:
41+
"""Return the configured worker isolation mode.
42+
43+
Reads ``CONDUCTOR_WORKER_ISOLATION`` (case-insensitive, whitespace
44+
trimmed). Invalid values warn once and fall back to ``process`` — failing
45+
open to the default behavior rather than bricking every worker on a typo.
46+
"""
47+
global _invalid_mode_warned
48+
raw = os.environ.get(WORKER_ISOLATION_ENV, "")
49+
mode = raw.strip().lower() or ISOLATION_PROCESS
50+
if mode not in _VALID_MODES:
51+
if not _invalid_mode_warned:
52+
logger.warning(
53+
"Ignoring invalid %s=%r; valid values are %s; using %r",
54+
WORKER_ISOLATION_ENV,
55+
raw,
56+
sorted(_VALID_MODES),
57+
ISOLATION_PROCESS,
58+
)
59+
_invalid_mode_warned = True
60+
mode = ISOLATION_PROCESS
61+
return mode
62+
63+
64+
class _ThreadAsProcess(threading.Thread):
65+
"""threading.Thread shim that satisfies the multiprocessing.Process interface."""
66+
67+
def __init__(
68+
self, group=None, target=None, name=None, args=(), kwargs=None, *, daemon=None
69+
):
70+
super().__init__(
71+
group=group, target=target, name=name, args=args, kwargs=kwargs or {}, daemon=daemon
72+
)
73+
self.exitcode: Any = None
74+
75+
def terminate(self) -> None:
76+
pass
77+
78+
def kill(self) -> None:
79+
pass
80+
81+
@property
82+
def pid(self) -> None:
83+
return None
84+
85+
86+
def apply_thread_isolation() -> None:
87+
"""Swap ``task_handler``'s multiprocessing primitives for thread equivalents.
88+
89+
Idempotent. Must run before the first :class:`TaskHandler` is constructed:
90+
its ``__init__`` creates the logging ``Queue`` and starts the logger
91+
``Process`` immediately.
92+
"""
93+
from conductor.client.automator import task_handler as _th_module # lazy: no import cycle
94+
95+
if getattr(_th_module, "_thread_isolation_applied", False):
96+
return
97+
98+
_th_module.Process = _ThreadAsProcess # type: ignore[attr-defined]
99+
100+
# With every worker a thread, the log relay is same-process: a plain
101+
# thread-safe queue replaces the IPC Queue, whose SemLock would drag in
102+
# the resource_tracker bootstrap this mode exists to avoid.
103+
_th_module.Queue = queue.Queue # type: ignore[attr-defined]
104+
105+
# The worker/logger process targets call signal.signal(SIGINT, SIG_IGN)
106+
# at startup — valid in a child process but a ValueError in a non-main
107+
# thread. Make signal.signal skip silently off the main thread.
108+
import signal as _signal_mod
109+
110+
_orig_signal_fn = _signal_mod.signal
111+
112+
def _thread_safe_signal(signalnum, handler):
113+
if threading.current_thread() is threading.main_thread():
114+
return _orig_signal_fn(signalnum, handler)
115+
# Non-main thread: skip silently
116+
117+
_signal_mod.signal = _thread_safe_signal # type: ignore[assignment]
118+
119+
_th_module._thread_isolation_applied = True # type: ignore[attr-defined]
Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
"""Unit tests for conductor.client.automator.worker_isolation.
2+
3+
Every test that applies the patch restores the touched globals afterwards
4+
(task_handler.Process / task_handler.Queue, signal.signal, the idempotency
5+
flag): other suites — e.g. test_task_handler.test_start_processes — assert
6+
against the real multiprocessing primitives and must not see a patched world.
7+
"""
8+
9+
import multiprocessing
10+
import queue
11+
import signal
12+
import threading
13+
14+
import pytest
15+
16+
from conductor.client.automator import task_handler, worker_isolation
17+
from conductor.client.automator.worker_isolation import (
18+
ISOLATION_PROCESS,
19+
ISOLATION_THREAD,
20+
WORKER_ISOLATION_ENV,
21+
_ThreadAsProcess,
22+
apply_thread_isolation,
23+
isolation_mode,
24+
)
25+
26+
27+
@pytest.fixture
28+
def restore_globals():
29+
"""Snapshot and restore everything apply_thread_isolation() mutates."""
30+
orig_process = task_handler.Process
31+
orig_queue = task_handler.Queue
32+
orig_signal = signal.signal
33+
had_flag = getattr(task_handler, "_thread_isolation_applied", False)
34+
orig_warned = worker_isolation._invalid_mode_warned
35+
yield
36+
task_handler.Process = orig_process
37+
task_handler.Queue = orig_queue
38+
signal.signal = orig_signal
39+
if had_flag:
40+
task_handler._thread_isolation_applied = True
41+
elif hasattr(task_handler, "_thread_isolation_applied"):
42+
del task_handler._thread_isolation_applied
43+
worker_isolation._invalid_mode_warned = orig_warned
44+
45+
46+
# ── isolation_mode() ─────────────────────────────────────────────────────────
47+
48+
49+
def test_default_when_unset(monkeypatch):
50+
monkeypatch.delenv(WORKER_ISOLATION_ENV, raising=False)
51+
assert isolation_mode() == ISOLATION_PROCESS
52+
53+
54+
def test_explicit_process(monkeypatch):
55+
monkeypatch.setenv(WORKER_ISOLATION_ENV, "process")
56+
assert isolation_mode() == ISOLATION_PROCESS
57+
58+
59+
def test_thread_case_insensitive_and_trimmed(monkeypatch):
60+
for value in ("thread", "THREAD", " Thread "):
61+
monkeypatch.setenv(WORKER_ISOLATION_ENV, value)
62+
assert isolation_mode() == ISOLATION_THREAD
63+
64+
65+
def test_empty_value_means_default(monkeypatch):
66+
monkeypatch.setenv(WORKER_ISOLATION_ENV, " ")
67+
assert isolation_mode() == ISOLATION_PROCESS
68+
69+
70+
def test_invalid_value_warns_once_and_falls_back(monkeypatch, caplog, restore_globals):
71+
worker_isolation._invalid_mode_warned = False
72+
monkeypatch.setenv(WORKER_ISOLATION_ENV, "threads")
73+
with caplog.at_level("WARNING", logger=worker_isolation.logger.name):
74+
assert isolation_mode() == ISOLATION_PROCESS
75+
assert isolation_mode() == ISOLATION_PROCESS # second call: no new warning
76+
warnings = [r for r in caplog.records if WORKER_ISOLATION_ENV in r.getMessage()]
77+
assert len(warnings) == 1
78+
assert "threads" in warnings[0].getMessage()
79+
80+
81+
# ── default world (G1, module level) ─────────────────────────────────────────
82+
83+
84+
def test_module_import_has_no_side_effects():
85+
"""Importing worker_isolation must not patch anything by itself."""
86+
assert task_handler.Process is multiprocessing.Process
87+
assert task_handler.Queue is multiprocessing.Queue
88+
assert not getattr(task_handler, "_thread_isolation_applied", False)
89+
90+
91+
# ── apply_thread_isolation() ─────────────────────────────────────────────────
92+
93+
94+
def test_apply_swaps_process_and_queue(restore_globals):
95+
apply_thread_isolation()
96+
assert task_handler.Process is _ThreadAsProcess
97+
assert task_handler.Queue is queue.Queue
98+
assert task_handler._thread_isolation_applied is True
99+
100+
101+
def test_apply_is_idempotent(restore_globals):
102+
apply_thread_isolation()
103+
signal_after_first = signal.signal
104+
apply_thread_isolation()
105+
# A second call must not re-wrap signal.signal (or anything else).
106+
assert signal.signal is signal_after_first
107+
assert task_handler.Process is _ThreadAsProcess
108+
109+
110+
def test_signal_noop_off_main_thread(restore_globals):
111+
apply_thread_isolation()
112+
113+
# Off the main thread: the exact call every worker target makes at
114+
# startup must not raise ValueError.
115+
errors = []
116+
117+
def target():
118+
try:
119+
signal.signal(signal.SIGINT, signal.SIG_IGN)
120+
except Exception as exc: # pragma: no cover - failure path
121+
errors.append(exc)
122+
123+
t = threading.Thread(target=target)
124+
t.start()
125+
t.join()
126+
assert errors == []
127+
128+
# On the main thread: still delegates to the real signal.signal.
129+
current = signal.getsignal(signal.SIGINT)
130+
assert signal.signal(signal.SIGINT, current) is current
131+
132+
133+
# ── _ThreadAsProcess shim interface ──────────────────────────────────────────
134+
135+
136+
def test_shim_satisfies_process_interface():
137+
ran = threading.Event()
138+
p = _ThreadAsProcess(target=ran.set, daemon=True)
139+
assert p.pid is None
140+
assert p.exitcode is None
141+
p.start()
142+
p.join(timeout=5)
143+
assert ran.is_set()
144+
assert not p.is_alive()
145+
# Process-interface methods exist and are safe no-ops.
146+
p.terminate()
147+
p.kill()
148+
149+
150+
def test_shim_passes_args_and_kwargs():
151+
seen = {}
152+
153+
def target(a, b=None):
154+
seen["a"] = a
155+
seen["b"] = b
156+
157+
p = _ThreadAsProcess(target=target, args=(1,), kwargs={"b": 2}, daemon=True)
158+
p.start()
159+
p.join(timeout=5)
160+
assert seen == {"a": 1, "b": 2}

0 commit comments

Comments
 (0)