fix: propagate ContextVar state to async_execution worker threads#4834
fix: propagate ContextVar state to async_execution worker threads#4834SinzoL wants to merge 1 commit intocrewAIInc:mainfrom
Conversation
Task.execute_async() spawns a new threading.Thread for background execution, but threading.Thread does not inherit the parent's contextvars.Context. This causes ContextVar values (e.g. tracing spans, tenant IDs, session state) to be lost in the worker thread. Use contextvars.copy_context() to snapshot the current context and run the worker function inside it via ctx.run(). Fixes crewAIInc#4822
alvinttang
left a comment
There was a problem hiding this comment.
Review
This PR fixes ContextVar state not propagating to worker threads spawned by Task.execute_async(). The fix uses contextvars.copy_context() to snapshot the current context and ctx.run() to execute the task function within that snapshot.
Analysis:
-
The fix is correct and idiomatic:
contextvars.copy_context()+ctx.run()is the standard Python pattern for propagatingContextVarstate to threads. This is well-documented in the Python stdlib and is the right approach. -
ctx.runsignature:ctx.run(callable, *args)— here it'sctx.run(self._execute_task_async, agent, context, tools, future). This correctly passes all positional arguments through. Good. -
Thread safety of the context copy:
copy_context()creates a shallow copy of the current context at the time of the call. AnyContextVar.set()calls in the worker thread will only affect the copy, not the parent. This is the desired behavior — each async task execution gets an isolated copy of the context snapshot. No race conditions here. -
Interaction with
Future: Thefutureobject is still shared between the parent and worker thread (it's passed as an argument, not a ContextVar). This is correct — theFutureneeds to be the same object so the parent can read the result. -
Import placement:
import contextvarsis added at module top level alongsidethreading, which is correct. No lazy import needed sincecontextvarsis a stdlib module with negligible import cost. -
Broader applicability: Are there other places in crewAI that spawn threads without propagating context? A quick search for
threading.Threadacross the codebase might reveal similar issues. If so, this pattern should be applied consistently. -
No test: Testing ContextVar propagation is straightforward — set a ContextVar in the parent, run
execute_async, assert the var is visible in the task function. Would be a valuable regression test.
This is a clean, correct, minimal fix. The only suggestion is to check for other thread-spawning sites that might need the same treatment and to add a test.
|
This PR is stale because it has been open for 45 days with no activity. |
Task.execute_async() spawns a new threading.Thread for background execution, but threading.Thread does not inherit the parent's contextvars.Context. This causes ContextVar values (e.g. tracing spans, tenant IDs, session state) to be lost in the worker thread. Use contextvars.copy_context() to snapshot the current context and run the worker function inside it via ctx.run(). Fixes #4822
Note
Medium Risk
Touches core async task execution by changing how worker threads are started, which could affect any code relying on thread startup semantics. The change is small but impacts tracing/tenant context propagation across all
Task.execute_asynccalls.Overview
Fixes lost
ContextVarstate in async task execution.Task.execute_async()now snapshots the currentcontextvarscontext and starts the background thread viactx.run(...), ensuring context-local values (e.g., tracing/tenant/session data) propagate into the worker thread.Written by Cursor Bugbot for commit 06973d3. This will update automatically on new commits. Configure here.