Skip to content

Carry the runtime Context into thread pool workers - #4

Open
tamohannes wants to merge 1 commit into
nustackdev:mainfrom
tamohannes:fix/parallel-ctx-propagation
Open

Carry the runtime Context into thread pool workers#4
tamohannes wants to merge 1 commit into
nustackdev:mainfrom
tamohannes:fix/parallel-ctx-propagation

Conversation

@tamohannes

Copy link
Copy Markdown

make test is red on main: 5 failures, all the same error.

LookupError: <ContextVar name='nu_rt_ctx'>
  src/nu/lang/runtime/runtime.py:75  in Runtime.ctx

Cause

Runtime.ctx resolves through the _RT_CTX ContextVar, which the comment above it describes as per asyncio task with copy on write inheritance. That holds for tasks. It does not hold for threads: a ThreadPoolExecutor worker starts with an empty contextvars context, so _RT_CTX is simply unset there.

Every hand-off to the Budget's pool goes over that boundary bare:

futures = [pool.submit(self.eval, n) for n in nids]          # eval_parallel
return await loop.run_in_executor(pool, self.eval, n)        # _drive_async

So a parallel branch dies as soon as it touches the Context, which is what ref._write does on the way into ctx.attrs.

Regression from 322f9f3 ("Make Runtime.ctx per-asyncio-task via ContextVar"), 2026-07-21.

Fix

Take a fresh contextvars.copy_context() on the calling side and submit through its run. Six dispatch sites: eval_parallel, _drive_async, merge, amerge, in_thread, a_in_thread.

A copy rather than the caller's own context is deliberate, and it gives exactly the semantics the module comment already claims: the Context object is shared so writes to ctx.attrs still land where the caller can see them, while rebinding _RT_CTX inside a branch stays local to that branch. It also avoids the "cannot enter context: already entered" case, since two workers never share one Context object.

Verification

Before: 5 failed, 2053 passed. After: 2058 passed, 27 skipped.

Added one regression test per dispatch site under a new "context across the thread boundary" section in tests/nu/lang/runtime/test_runtime.py. merge, amerge, in_thread and a_in_thread had no coverage for this at all. All six fail on main and pass here.

Found while checking #2 on a clean clone, so this is what make test does out of the box, not a code read.

🤖 Generated with Claude Code

`Runtime.ctx` resolves through the `_RT_CTX` ContextVar. A pool worker starts
with an empty contextvars context, so any branch dispatched through
`pool.submit(...)` or `loop.run_in_executor(...)` raised
`LookupError: <ContextVar name='nu_rt_ctx'>` the moment it touched the Context.

Every dispatch site now submits through a fresh `contextvars.copy_context()`
taken on the calling side: `eval_parallel`, `_drive_async`, `merge`, `amerge`,
`in_thread` and `a_in_thread`. The Context stays resolvable on the worker, and
a copy per branch keeps a `.set()` inside a branch local to it, which is the
same copy-on-write rule an asyncio.Task already gets.

This turns 5 failing tests on main green (test_operators, test_strategy,
test_eval_modes_e2e) and adds one regression test per dispatch site, covering
`merge`, `amerge`, `in_thread` and `a_in_thread`, which had no test for this.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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