Skip to content

fix(OMN-13658): sync event_publisher loop affinity — schedule onto kernel loop via run_coroutine_threadsafe - #2133

Merged
jonahgabriel merged 1 commit into
devfrom
jonah/omn-13658-runtime-effect-publish-path-asyncio-event-loop-affinity-bug
Jun 28, 2026
Merged

jonahgabriel merged 1 commit into
devfrom
jonah/omn-13658-runtime-effect-publish-path-asyncio-event-loop-affinity-bug

Conversation

@jonahgabriel

@jonahgabriel jonahgabriel commented Jun 28, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Fixes the asyncio event-loop affinity bug in the auto-wired sync event_publisher
adapter (src/omnibase_infra/runtime/auto_wiring/handler_wiring.py,
_make_sync_event_publisher).

The dispatch engine offloads blocking sync handlers (e.g.
HandlerContextRoiRunner, node_context_roi_runner) onto a ThreadPoolExecutor
worker thread via run_in_executor. When such a handler published a terminal
event, the old adapter called asyncio.get_running_loop(), hit RuntimeError
(worker threads own no loop), and fell back to asyncio.run(...) — spinning a
throwaway event loop in the worker thread.

The publish awaitable returned by the event bus binds its internal Futures to the
runtime kernel loop. Running it on that foreign worker-thread loop produced
the got Future attached to a different loop warning and a 2-3 minute
terminal-emission retry delay (attempt 1/4 ... retry), plus duplicate delivery.

Fix

  • Capture the runtime kernel loop at construction time — wire_from_manifest
    builds the publisher while running on the kernel loop, so
    asyncio.get_running_loop() resolves to the kernel loop and is closed over.
  • On publish from any thread that does not own the kernel loop, schedule the
    coroutine back onto the kernel loop with
    asyncio.run_coroutine_threadsafe(publish_awaitable, kernel_loop). Every Future
    stays on its owning loop, so the publish completes immediately.
  • Publishes already on the kernel loop's own thread (async handler path) keep the
    create_task fast path.
  • The asyncio.run(...) throwaway-loop branch is removed entirely — no new loop
    is ever spawned.

Tests

tests/unit/runtime/auto_wiring/test_sync_event_publisher_loop_affinity.py:

  • test_sync_publisher_from_worker_thread_runs_on_kernel_loop — runs the kernel
    loop in a background thread, builds the publisher on that loop, then calls the
    sync publisher from a separate worker thread. Asserts it completes without
    error, never calls asyncio.run (patched + assert_not_called), and the
    publish coroutine executes on the kernel loop (publish_loop is kernel_loop)
    — not a worker-thread loop.
  • test_sync_publisher_from_kernel_loop_thread_schedules_task — the on-loop
    (async handler) path still delivers via create_task.

TDD: the worker-thread test fails on the pre-fix code (asyncio.run called once),
passes on the fix.

Verification

  • uv run pytest tests/ -m unit — 21480 passed, 22 skipped.
  • uv run pytest tests/unit/runtime/auto_wiring/ tests/integration/runtime/ ... —
    green (DB-backed integration tests skip locally; CI has the DB).
  • uv run mypy src/omnibase_infra/runtime/ --strict — clean (276 files).
  • ruff format + ruff check clean; pre-commit hooks pass on changed files.

DoD #3 (live dev-lane repro on node_context_roi_runner showing no attempt-1/4
retry warning + no 2-3 min terminal delay) is post-merge: this wiring-only fix is
proven pre-merge by the deterministic loop-affinity unit test; the live dev-lane
redeploy/repro is performed after merge. No prod/stability/.201 mutation.

Evidence

Evidence-Source: OCC#3238
Evidence-Ticket: OMN-13658

Closes OMN-13658.

…n_coroutine_threadsafe

_make_sync_event_publisher captured no owning loop and, when invoked from a
ThreadPoolExecutor worker thread (the dispatch engine offloads blocking sync
handlers via run_in_executor), fell back to asyncio.run() which spun a throwaway
loop in the worker thread. The publish awaitable's internal Futures are bound to
the kernel loop, so running them on that foreign loop raised 'got Future attached
to a different loop' and delayed terminal emission 2-3 min via retry.

Capture the runtime kernel loop at construction time (wire_from_manifest runs on
it) and, for publishes from any non-kernel thread, schedule the coroutine back
onto the kernel loop with asyncio.run_coroutine_threadsafe. Publishes already on
the kernel loop keep the create_task fast path. No new asyncio.run loop is ever
spawned.

Adds tests/unit/runtime/auto_wiring/test_sync_event_publisher_loop_affinity.py
proving a worker-thread publish completes without error, never calls
asyncio.run, and runs the coroutine on the kernel loop.
@coderabbitai

coderabbitai Bot commented Jun 28, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

_make_sync_event_publisher now captures the kernel event loop at wiring time via asyncio.get_running_loop(), raises ModelOnexError if none exists, and schedules publish coroutines using create_task when called from the kernel thread or run_coroutine_threadsafe from worker threads, removing the previous asyncio.run fallback. New regression tests verify both scheduling paths.

Sync Event Publisher Loop Affinity Fix

Layer / File(s) Summary
Loop capture and cross-thread scheduling
src/omnibase_infra/runtime/auto_wiring/handler_wiring.py
Adds import concurrent.futures; expands docstring; captures kernel_loop at wiring time (raises ModelOnexError if no loop); updates _log_publish_failure to accept both future types; branches on call-site thread to use create_task or run_coroutine_threadsafe.
Loop affinity regression tests
tests/unit/runtime/auto_wiring/test_sync_event_publisher_loop_affinity.py
Adds two tests using a recording event bus: worker-thread path patches asyncio.run to assert it is never called and verifies publish ran on the kernel loop; kernel-thread path verifies publish ran on the currently running loop.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 No more foreign loops for me,
I bind my tasks where they should be!
Kernel thread? create_task with glee.
Worker thread? threadsafe is the key.
The future lands where it was born — hooray! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main fix: preserving sync event_publisher loop affinity by scheduling work onto the kernel loop.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jonah/omn-13658-runtime-effect-publish-path-asyncio-event-loop-affinity-bug

Comment @coderabbitai help to get the list of available commands.

@jonahgabriel
jonahgabriel enabled auto-merge June 28, 2026 06:57
@jonahgabriel
jonahgabriel added this pull request to the merge queue Jun 28, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a manual request Jun 28, 2026
@jonahgabriel jonahgabriel reopened this Jun 28, 2026
@jonahgabriel
jonahgabriel enabled auto-merge June 28, 2026 07:15
@jonahgabriel
jonahgabriel added this pull request to the merge queue Jun 28, 2026
@jonahgabriel
jonahgabriel removed this pull request from the merge queue due to a manual request Jun 28, 2026
@jonahgabriel
jonahgabriel added this pull request to the merge queue Jun 28, 2026
Merged via the queue into dev with commit 1cb5fe8 Jun 28, 2026
202 of 207 checks passed
@jonahgabriel
jonahgabriel deleted the jonah/omn-13658-runtime-effect-publish-path-asyncio-event-loop-affinity-bug branch June 28, 2026 08:32
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