Skip to content

fix(core): make OGXAsLibraryClient thread-safe - #5773

Merged
franciscojavierarceo merged 1 commit into
ogx-ai:mainfrom
zy1o:zy1o_ogx_sync_client
May 11, 2026
Merged

fix(core): make OGXAsLibraryClient thread-safe#5773
franciscojavierarceo merged 1 commit into
ogx-ai:mainfrom
zy1o:zy1o_ogx_sync_client

Conversation

@zy1o

@zy1o zy1o commented May 10, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Rework the sync-on-async client.

The previous implementation called loop.run_until_complete() on a shared loop instance inside .request() method. When multiple threads called request() concurrently, this raised RuntimeError: This event loop is already running.

The fix uses a single dedicated daemon thread running loop.run_forever(), with all coroutines submitted with a asyncio.run_coroutine_threadsafe() call.

Known limitations and tradeoffs

  • Not fully thread-safe: per @mattf's comment on OGXAsLibraryClient is not thread-safe #5752, global state in the async client may still cause races under concurrent load. This PR fixes the event loop collision specifically.
  • Uses a bounded threading.Queue with busy-poll fallback (asyncio.sleep(0.01)) when full. My judgement call is: This is good enough for LLM token rates.
  • future.cancel() on the streaming coroutine is best-effort; a coroutine blocked on network I/O won't respond immediately.
  • Decided against the use of janus to avoid extra dependencies.

Closes #5752

Test Plan

Added unit tests covering:

  • Concurrent requests from multiple threads.
  • Init failure cleanup (background thread stops on failed initialization).

Launch just them with:

pytest tests/unit/ -vs -k TestOGXAsLibraryClientSyncOnAsync

@franciscojavierarceo
franciscojavierarceo added this pull request to the merge queue May 11, 2026
Merged via the queue into ogx-ai:main with commit 58b31a4 May 11, 2026
39 checks passed
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.

OGXAsLibraryClient is not thread-safe

3 participants