Skip to content

Commit dfadadd

Browse files
changliu2Copilot
andcommitted
fix(otel): use asyncio.Lock per loop in LiveOTelExporter to avoid event-loop deadlock
The rollout stage runs all OTelTracedSession instances in a single event loop (`asyncio.run` -> `rollout.execute`). Holding a sync `threading.Lock` across the inner `await invoke_callable(...)` in `OTelTracedSession.run_turn` blocked the entire loop: when `rollout.concurrency > 1`, the second coroutine could not acquire the lock because the first was awaiting an HTTP call on the same loop, and the first could never resume because the loop thread was parked on the sync lock. Reproducer (pre-fix): `rollout.concurrency: 4` with the LangGraph travel-planner callable target deadlocked indefinitely after the first batch of concurrent rollouts. Heartbeat froze and no transcripts were produced. Fix: - `LiveOTelExporter._lock` becomes an `asyncio.Lock` created lazily per running event loop via `LiveOTelExporter.get_lock()`. This serializes the clear-invoke-export cycle within one loop while yielding the loop to other coroutines while waiting. - `OTelTracedSession.run_turn` switches from `lock.acquire()` / `finally lock.release()` to `async with lock_ctx`, using `contextlib.nullcontext()` when `live_otel` is False. - A class-level `asyncio.Lock()` would bind to the first loop on use and raise on subsequent `asyncio.run` calls; the per-loop cache keeps repeated runs and tests safe. Verification: - `uv run pytest -q` -> 505 passed, 14 skipped. - LangGraph travel-planner config patched to `rollout.concurrency: 4` with 20 seeds end-to-end: PASS in 375s, 20 transcripts, 20 scores. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent b7f063e commit dfadadd

2 files changed

Lines changed: 31 additions & 14 deletions

File tree

p2m/core/otel.py

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@
1515

1616
from __future__ import annotations
1717

18+
import asyncio
1819
import json
19-
import threading
2020
from dataclasses import dataclass, field
2121
from pathlib import Path
2222
from typing import Any, Protocol, runtime_checkable
@@ -451,7 +451,27 @@ class LiveOTelExporter:
451451
_instance: "LiveOTelExporter | None" = None
452452
_setup_done: bool = False
453453
_sdk_exporter: Any = None
454-
_lock: threading.Lock = threading.Lock()
454+
# Per-event-loop async lock, created lazily on first use. A class-level
455+
# ``asyncio.Lock()`` would bind to whichever loop happened to be running
456+
# at first use and then raise in any subsequent ``asyncio.run()``. We
457+
# cache (loop, lock) and recreate when the loop changes so:
458+
# - within one rollout (one event loop), all concurrent sessions share
459+
# the same lock and serialize the clear-invoke-export cycle;
460+
# - tests / repeated runs that create new event loops still work.
461+
# NOTE: ``threading.Lock`` MUST NOT be used here — it would block the
462+
# entire event loop across the inner ``await`` and deadlock when
463+
# ``rollout.concurrency > 1``.
464+
_lock: asyncio.Lock | None = None
465+
_lock_loop: asyncio.AbstractEventLoop | None = None
466+
467+
@classmethod
468+
def get_lock(cls) -> asyncio.Lock:
469+
"""Return an ``asyncio.Lock`` bound to the current running loop."""
470+
loop = asyncio.get_running_loop()
471+
if cls._lock is None or cls._lock_loop is not loop:
472+
cls._lock = asyncio.Lock()
473+
cls._lock_loop = loop
474+
return cls._lock
455475

456476
def __new__(cls) -> "LiveOTelExporter":
457477
if cls._instance is None:

p2m/core/otel_session.py

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@
1919

2020
import importlib
2121
import inspect
22-
import threading
2322
import uuid
23+
from contextlib import nullcontext
2424
from typing import Any
2525

2626
from p2m.core.async_utils import invoke_callable
@@ -141,16 +141,16 @@ async def run_turn(self, messages: list[Message]) -> TurnResult:
141141
- ``span.id``: Each span → its own event (maximum granularity)
142142
143143
When using ``LiveOTelExporter`` (singleton, shared span buffer), the
144-
clear-invoke-export cycle is serialized via a lock to prevent
145-
concurrent sessions from contaminating each other's span data.
144+
clear-invoke-export cycle is serialized via an ``asyncio.Lock`` to
145+
prevent concurrent sessions from contaminating each other's span
146+
data. The lock MUST be ``asyncio.Lock`` (not ``threading.Lock``):
147+
the rollout stage runs all sessions in one event loop, so holding a
148+
sync threading lock across the inner ``await`` would block the loop
149+
and deadlock when ``rollout.concurrency > 1``.
146150
"""
147-
# Acquire the live exporter lock for the entire clear-invoke-export
148-
# cycle so concurrent OTelTracedSessions don't mix spans.
149151
from p2m.core.otel import LiveOTelExporter
150-
lock = LiveOTelExporter._lock if self._live_otel else None
151-
if lock:
152-
lock.acquire()
153-
try:
152+
lock_ctx = LiveOTelExporter.get_lock() if self._live_otel else nullcontext()
153+
async with lock_ctx:
154154
# Clear spans from previous turn so we only capture this turn's execution
155155
if self._live_exporter is not None:
156156
self._live_exporter.clear()
@@ -188,9 +188,6 @@ async def run_turn(self, messages: list[Message]) -> TurnResult:
188188

189189
# Collect traces for this turn
190190
turn_spans = self._exporter.export_session(turn_id)
191-
finally:
192-
if lock:
193-
lock.release()
194191

195192
validation = validate_spans(turn_spans)
196193

0 commit comments

Comments
 (0)