From 830ed2909e2cad7f4fa5e63efb040bf0e8f8b12f Mon Sep 17 00:00:00 2001 From: VascoSch92 Date: Wed, 27 May 2026 11:49:52 +0200 Subject: [PATCH] docs(sdk): document FIFOLock held across arun()'s await arun() holds the state lock (FIFOLock) across 'await astep'. That is safe only because every other state mutator is dispatched via run_in_executor onto a worker thread; FIFOLock is reentrant by OS thread id, so a state-mutating coroutine awaited on the event-loop thread would silently re-enter the lock and corrupt history. Make this load-bearing invariant explicit with a comment at the await site and a note on FIFOLock. Comment-only; no behavior change. --- openhands-sdk/openhands/sdk/conversation/fifo_lock.py | 5 +++++ .../openhands/sdk/conversation/impl/local_conversation.py | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/openhands-sdk/openhands/sdk/conversation/fifo_lock.py b/openhands-sdk/openhands/sdk/conversation/fifo_lock.py index 56758b366b..fe780e06fb 100644 --- a/openhands-sdk/openhands/sdk/conversation/fifo_lock.py +++ b/openhands-sdk/openhands/sdk/conversation/fifo_lock.py @@ -24,6 +24,11 @@ class FIFOLock: - FIFO ordering: Threads get lock in request order - Context manager support: Use with 'with' statement - Thread-safe: Safe for concurrent access + + Note: reentrancy is keyed by OS thread id, not by asyncio task, so the + lock does NOT serialize concurrent coroutines on the same event-loop + thread - each re-enters it. Only hold it across an ``await`` when no + other task on that thread can acquire it. """ _mutex: threading.Lock diff --git a/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py b/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py index ab38d0d3d9..e00ac888f8 100644 --- a/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py +++ b/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py @@ -1005,6 +1005,13 @@ async def arun(self) -> None: ConversationExecutionStatus.RUNNING ) + # The state lock is held across this await. This is safe + # only because every other state mutator is dispatched via + # run_in_executor onto a worker thread: FIFOLock is thread- + # (not task-) reentrant, so a state-mutating coroutine + # awaited on this event-loop thread would silently re-enter + # the lock and corrupt history. Do not await state mutations + # on the event-loop thread. await self.agent.astep( self, on_event=self._on_event,