Skip to content

fix(ipc): surface thread runner initialization errors - #7939

Open
BlueX888 wants to merge 2 commits into
livekit:mainfrom
BlueX888:fix/thread-executor-init-error
Open

BlueX888 wants to merge 2 commits into
livekit:mainfrom
BlueX888:fix/thread-executor-init-error

Conversation

@BlueX888

@BlueX888 BlueX888 commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

fix(ipc): surface runner initialization errors

What breaks

Two gaps let a failed warm-up look successful:

  1. ThreadJobExecutor.initialize() asserts that the first IPC message is an InitializeResponse but ignores its error field. When the runner thread's initialize_process_fnc raises (setup hook, plugin prewarm, model download), _ProcClient.initialize sends InitializeResponse(error=...) and the thread exits, yet the executor resolves _initialize_fut successfully. ProcPool then queues the dead runner as warmed: a dispatched job is written into the still-open socket, its entrypoint never runs, and _main_task() unconditionally sets JobStatus.SUCCESS, so the worker reports JS_SUCCESS for a job that never executed. THREAD is the default executor on Windows and in dev mode.
  2. _ProcClient.initialize() reports InitializeResponse(error=str(e)). For exceptions with an empty string representation, such as a bare AssertionError, that is "", which every parent-side if init_res.error: check (ThreadJobExecutor, ProcJobExecutor, SupervisedProc) reads as success — so the same dead-runner warm-up could still happen for both executor types.

Root cause

SupervisedProc.initialize() already raises when init_res.error is set (supervised_proc.py:286-287); ThreadJobExecutor.initialize() is missing the equivalent check before resolving _initialize_fut, and the child IPC client can emit an empty error string that is indistinguishable from success.

Fix

  • job_thread_executor.py: after the InitializeResponse assert, raise RuntimeError(f"runner initialization failed: {init_res.error}") when the error is non-empty, before the "job runner initialized" log and before resolving the future. The existing except Exception branch records the exception on _initialize_fut and re-raises, mirroring the process executor. ProcPool then discards the dead runner and never warms it.
  • proc_client.py: send str(e) or repr(e) or type(e).__name__ as the initialize error. Non-empty messages are unchanged (protocol-compatible); empty exceptions become a non-empty representation such as AssertionError(). This protects both the threaded and process executor paths.

_main_task()'s unconditional JobStatus.SUCCESS is intentionally left unchanged: with these fixes a failed runner never receives a RunningJobInfo (running_job is None), so Worker._update_job_status returns early and the bogus JS_SUCCESS path is closed. Distinguishing "job completed" from "thread died before the entrypoint ran" would need a new runner-side signal and would change every normal thread shutdown.

Tests

tests/test_ipc.py, parametrized over a normal error message and a bare AssertionError() (empty str):

  • test_thread_initialize_propagates_runner_error — a failing initialize_process_fnc makes initialize() raise RuntimeError("runner initialization failed: ...") and leaves the exception on _initialize_fut; the empty case reports runner initialization failed: AssertionError().
  • test_proc_pool_thread_does_not_warm_a_failed_runner — a ProcPool with JobExecutorType.THREAD and failing init raises RuntimeError("no process became available ...") from launch_job(), the entrypoint never runs, and no executor remains tracked.

On base 058db78e: all four fail with Failed: DID NOT RAISE RuntimeError (4 failed). With the fix: 4 passed; tests/test_ipc.py 21 passed; tests/test_ipc_preload.py tests/test_supervised_proc_memory.py 23 passed, 1 skipped; ruff clean.

The change touches the initialize() error path and the shared child IPC client; it does not alter the aclose()/monitor hunks of the in-flight #7899.

ThreadJobExecutor.initialize() asserted the first IPC message was an
InitializeResponse but ignored its error field. When the runner's
initialize_process_fnc raised, the runner reported the failure and exited,
yet the executor resolved initialization successfully and ProcPool queued
the dead runner as warmed. A job dispatched to it was written into a dead
socket, its entrypoint never ran, and _main_task() still marked the
executor SUCCESS, so the worker reported JS_SUCCESS for a job that never
executed.

Mirror SupervisedProc.initialize(): raise
"runner initialization failed: <error>" when the response carries an
error, before resolving _initialize_fut. The pool now discards the runner
and launch_job fails after its retries instead of claiming the job ran.

Add regression tests for the direct executor and for ProcPool with the
THREAD executor.
@BlueX888
BlueX888 requested a review from a team as a code owner October 19, 2026 05:32

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Devin Review

Comment on lines +190 to +191
if init_res.error:
raise RuntimeError(f"runner initialization failed: {init_res.error}")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Empty errors warm dead runners

When prewarm raises AssertionError() or another exception with empty text, init_res.error passes as success. _ProcClient sends the empty error and exits, so the pool can dispatch jobs to a dead runner.

Learn more

The runner turns every prewarm exception into an InitializeResponse using str(e) in _ProcClient.initialize. An exception with no message produces the same empty error as a successful response. The parent therefore cannot distinguish the two outcomes from the response alone and treats a dead thread as initialized. ProcPool._proc_spawn_task then queues it for jobs.

Example: A prewarm hook executes assert model is not None without a custom message. When it fails, str(AssertionError()) is empty; the thread exits, but initialization succeeds and a job can be assigned to that thread.

Recommended fix: Make the sender guarantee a nonempty error for every caught exception, such as a fallback to its type name, or add a separate success/failure field to InitializeResponse. Test an exception with empty str(e) through both direct initialization and pool dispatch.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

_ProcClient.initialize() sent InitializeResponse(error=str(e)). For
exceptions with an empty str(), such as a bare AssertionError raised by
initialize_process_fnc, that produced an empty error string, which
ProcJobExecutor, SupervisedProc and ThreadJobExecutor all treat as a
successful initialization. The pool warmed the dead runner and could
dispatch a job whose entrypoint never ran.

Send str(e) when it has content to keep existing messages unchanged, and
fall back to repr(e) (then the exception type name) otherwise, so the
parent always sees a non-empty failure. Add parametrized regression
coverage in tests/test_ipc.py for a prewarm raising AssertionError():
initialize() must raise and ProcPool must not warm the runner.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant