Skip to content

Commit c455938

Browse files
Kowserclaude
andcommitted
feat(automator): CONDUCTOR_WORKER_ISOLATION=thread worker isolation mode
TaskHandler.__init__ applies thread isolation (before the logging Queue / logger Process are created) when the env var selects it; the Windows-only worker_manager shim becomes a delegating alias to the shared implementation. Default (process) path is byte-identical — spawn remains the start method. Fixes tool workers never polling inside Firecracker microVM guests, where multiprocessing's fork+exec bootstraps (spawn children, resource_tracker) fail. Verified E2E in the AgentSpan runtime stack. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 47584e2 commit c455938

5 files changed

Lines changed: 125 additions & 66 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Added
1111

12+
- Worker isolation mode: `CONDUCTOR_WORKER_ISOLATION=thread` runs every worker as a thread instead of a `multiprocessing.Process` (default `process` is unchanged — `spawn` remains the start method). For environments where multiprocessing's fork+exec bootstraps (spawn children, `resource_tracker`) fail, e.g. Firecracker microVM guests. Thread-mode tradeoffs: no per-worker force-kill (shutdown is cooperative), CPU-bound workers share the GIL, and `signal.signal` becomes a no-op off the main thread. Implementation: the Windows-only Process→Thread shim moved to `conductor.client.automator.worker_isolation` (the old `worker_manager._patch_conductor_use_threads_on_windows` remains as a delegating alias) and now also swaps the logging-relay `Queue` for a plain `queue.Queue`
13+
1214
- Canonical metrics mode: opt-in harmonized metric surface via `WORKER_CANONICAL_METRICS=true` -- [details](METRICS.md#detailed-technical-notes--unreleased)
1315
- `MetricsSettings` gains `clean_directory` and `clean_dead_pids` for opt-in stale `.db` file cleanup (both default to `False`)
1416
- `SchedulerClient` now carries the schedule lifecycle operations itself: `pause(reason=)`, `resume`, `delete`, `run_now`, `preview_next`, `reconcile` (declarative tri-state sync) — with typed errors (`ScheduleNotFound`, `InvalidCronExpression`, ...). `pause_schedule` gains an optional `reason=` (stored by OSS Conductor servers; ignored by Orkes servers)

src/conductor/ai/agents/runtime/worker_manager.py

Lines changed: 8 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -16,78 +16,20 @@
1616
import threading
1717
from typing import TYPE_CHECKING, Any, Optional
1818

19+
from conductor.client.automator.worker_isolation import apply_thread_isolation
20+
1921
logger = logging.getLogger("conductor.ai.agents.worker_manager")
2022

2123

2224
def _patch_conductor_use_threads_on_windows() -> None:
23-
"""On Windows, replace multiprocessing.Process with threading.Thread for Conductor workers.
24-
25-
Windows multiprocessing uses 'spawn' which requires all objects passed to
26-
child processes to be picklable. Conductor workers hold threading locks
27-
and closures that are not picklable. Using threads instead of processes
28-
sidesteps the entire issue: threads share the parent's memory so no
29-
pickling is needed, and tool functions are typically I/O-bound so the GIL
30-
is not a bottleneck.
25+
"""Deprecated alias — kept for existing callers.
3126
32-
Also patches the worker target functions to skip signal.signal() calls,
33-
which are forbidden in non-main threads.
27+
The Process->Thread shim (plus the Queue->queue.Queue half it was
28+
missing) now lives in :mod:`conductor.client.automator.worker_isolation`
29+
and is also activated by ``CONDUCTOR_WORKER_ISOLATION=thread``, read by
30+
:class:`TaskHandler` itself. See :func:`apply_thread_isolation`.
3431
"""
35-
try:
36-
from conductor.client.automator import task_handler as _th_module
37-
except ImportError:
38-
return
39-
40-
if getattr(_th_module, "_agentspan_thread_patched", False):
41-
return
42-
43-
# ── Thread shim ──────────────────────────────────────────────────────────
44-
45-
class _ThreadAsProcess(threading.Thread):
46-
"""threading.Thread shim that satisfies the multiprocessing.Process interface."""
47-
48-
def __init__(
49-
self, group=None, target=None, name=None, args=(), kwargs=None, *, daemon=None
50-
):
51-
super().__init__(
52-
group=group, target=target, name=name, args=args, kwargs=kwargs or {}, daemon=daemon
53-
)
54-
self.exitcode: Any = None
55-
56-
def terminate(self) -> None:
57-
pass
58-
59-
def kill(self) -> None:
60-
pass
61-
62-
@property
63-
def pid(self) -> None:
64-
return None
65-
66-
_th_module.Process = _ThreadAsProcess # type: ignore[attr-defined]
67-
68-
# ── Patch worker targets to skip signal.signal() in threads ──────────────
69-
# The conductor process targets call signal.signal(SIGINT, SIG_IGN) at the
70-
# top — valid in a real child process but raises ValueError in a thread.
71-
72-
import signal as _signal
73-
74-
# ── Patch signal.signal to be a no-op in non-main threads ────────────────
75-
# The conductor worker/logger process targets call signal.signal(SIGINT,
76-
# SIG_IGN) at startup — valid in a child process but raises ValueError
77-
# when called from a non-main thread. We monkey-patch signal.signal to
78-
# silently skip the call when not in the main thread.
79-
import signal as _signal_mod
80-
81-
_orig_signal_fn = _signal_mod.signal
82-
83-
def _thread_safe_signal(signalnum, handler):
84-
if threading.current_thread() is threading.main_thread():
85-
return _orig_signal_fn(signalnum, handler)
86-
# Non-main thread: skip silently
87-
88-
_signal_mod.signal = _thread_safe_signal # type: ignore[attr-defined]
89-
90-
_th_module._agentspan_thread_patched = True # type: ignore[attr-defined]
32+
apply_thread_isolation()
9133

9234

9335
if TYPE_CHECKING:

src/conductor/client/automator/task_handler.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
from conductor.client.automator.task_runner import TaskRunner
1616
from conductor.client.automator.async_task_runner import AsyncTaskRunner
17+
from conductor.client.automator import worker_isolation
1718
from conductor.client.configuration.configuration import Configuration
1819
from conductor.client.configuration.settings.metrics_settings import MetricsSettings
1920
from conductor.client.event.task_runner_events import TaskRunnerEvent
@@ -227,6 +228,11 @@ def __init__(
227228
restart_backoff_max_seconds: float = 60.0,
228229
restart_max_attempts: int = 0
229230
):
231+
# Thread isolation must be applied before _setup_logging_queue():
232+
# it creates the multiprocessing Queue and starts the logger Process —
233+
# the primitives CONDUCTOR_WORKER_ISOLATION=thread replaces.
234+
if worker_isolation.isolation_mode() == worker_isolation.ISOLATION_THREAD:
235+
worker_isolation.apply_thread_isolation()
230236
workers = workers or []
231237
self.logger_process, self.queue = _setup_logging_queue(configuration)
232238
self._configuration = configuration

tests/unit/ai/test_worker_manager.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,3 +356,34 @@ def test_filter_installed_on_conductor_logger(self):
356356
# Clean up
357357
for f in schema_filters:
358358
conductor_logger.removeFilter(f)
359+
360+
361+
class TestThreadPatchAlias:
362+
"""The Windows patch entry point now delegates to worker_isolation."""
363+
364+
def test_alias_delegates_to_apply_thread_isolation(self):
365+
import conductor.ai.agents.runtime.worker_manager as wm
366+
367+
with patch(
368+
"conductor.ai.agents.runtime.worker_manager.apply_thread_isolation"
369+
) as mock_apply:
370+
wm._patch_conductor_use_threads_on_windows()
371+
mock_apply.assert_called_once_with()
372+
373+
def test_windows_gate_calls_the_alias(self):
374+
import conductor.ai.agents.runtime.worker_manager as wm
375+
376+
config = MagicMock()
377+
manager = WorkerManager(configuration=config)
378+
with patch(
379+
"conductor.ai.agents.runtime.worker_manager.platform.system",
380+
return_value="Windows",
381+
), patch(
382+
"conductor.ai.agents.runtime.worker_manager._patch_conductor_use_threads_on_windows"
383+
) as mock_patch, patch(
384+
"conductor.client.automator.task_handler.TaskHandler"
385+
) as mock_th:
386+
mock_th.return_value.task_runner_processes = []
387+
mock_th.return_value.metrics_provider_process = None
388+
manager.start()
389+
mock_patch.assert_called_once_with()

tests/unit/automator/test_worker_isolation.py

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,15 @@
1010
import queue
1111
import signal
1212
import threading
13+
from unittest.mock import Mock, patch
1314

1415
import pytest
1516

1617
from conductor.client.automator import task_handler, worker_isolation
18+
from conductor.client.automator.task_handler import TaskHandler
19+
from conductor.client.automator.task_runner import TaskRunner
20+
from conductor.client.configuration.configuration import Configuration
21+
from tests.unit.resources.workers import ClassWorker
1722
from conductor.client.automator.worker_isolation import (
1823
ISOLATION_PROCESS,
1924
ISOLATION_THREAD,
@@ -158,3 +163,76 @@ def target(a, b=None):
158163
p.start()
159164
p.join(timeout=5)
160165
assert seen == {"a": 1, "b": 2}
166+
167+
168+
# ── TaskHandler gate (G2 / G5) ───────────────────────────────────────────────
169+
170+
171+
def _stop_logger(th):
172+
"""End the logger thread via its None sentinel (same as the SDK's atexit)."""
173+
th.queue.put(None)
174+
th.logger_process.join(timeout=2)
175+
176+
177+
def test_task_handler_construction_applies_thread_isolation(monkeypatch, restore_globals):
178+
monkeypatch.setenv(WORKER_ISOLATION_ENV, "thread")
179+
th = TaskHandler(
180+
configuration=Configuration(),
181+
workers=[],
182+
scan_for_annotated_workers=False,
183+
monitor_processes=False,
184+
)
185+
try:
186+
assert task_handler.Process is _ThreadAsProcess
187+
assert task_handler.Queue is queue.Queue
188+
assert isinstance(th.queue, queue.Queue)
189+
assert isinstance(th.logger_process, _ThreadAsProcess)
190+
finally:
191+
_stop_logger(th)
192+
193+
194+
def test_task_handler_default_mode_untouched(monkeypatch, restore_globals):
195+
monkeypatch.delenv(WORKER_ISOLATION_ENV, raising=False)
196+
th = TaskHandler(
197+
configuration=Configuration(),
198+
workers=[],
199+
scan_for_annotated_workers=False,
200+
monitor_processes=False,
201+
)
202+
try:
203+
assert task_handler.Process is multiprocessing.Process
204+
assert task_handler.Queue is multiprocessing.Queue
205+
assert not getattr(task_handler, "_thread_isolation_applied", False)
206+
assert isinstance(th.logger_process, multiprocessing.Process)
207+
finally:
208+
th.queue.put(None)
209+
th.logger_process.join(timeout=5)
210+
211+
212+
def test_thread_mode_start_stop_smoke(monkeypatch, restore_globals):
213+
"""G5: full worker lifecycle in thread mode — no signal ValueError."""
214+
monkeypatch.setenv(WORKER_ISOLATION_ENV, "thread")
215+
thread_errors = []
216+
monkeypatch.setattr(threading, "excepthook", lambda args: thread_errors.append(args))
217+
218+
with patch.object(TaskRunner, "run", Mock(return_value=None)):
219+
th = TaskHandler(
220+
configuration=Configuration(),
221+
workers=[ClassWorker("task")],
222+
scan_for_annotated_workers=False,
223+
monitor_processes=False,
224+
)
225+
try:
226+
th.start_processes()
227+
assert len(th.task_runner_processes) == 1
228+
for p in th.task_runner_processes:
229+
assert isinstance(p, _ThreadAsProcess)
230+
# run() is mocked to return, so the target must complete —
231+
# proving signal.signal + TaskRunner construction worked in
232+
# a non-main thread.
233+
p.join(timeout=5)
234+
assert not p.is_alive()
235+
th.stop_processes()
236+
finally:
237+
_stop_logger(th)
238+
assert thread_errors == []

0 commit comments

Comments
 (0)