fix(eventbus): only drain events in the waiting chain, not all buses (#5509) - #34
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
- P1: only drain events that are descendants of the awaited event (walk the parent chain), not siblings that merely share a parent id - P2: mark the queue task done after processing (finally), so wait_until_idle cannot return before a directly drained event finishes - P3: test now reproduces the documented mechanism deterministically: bus B's consumer is parked on the global lock while a second event is queued, so only the drain loop can reach it; assertion checks both bus B events are handled after the drain
|
Thanks for the review — all three points addressed in the latest push: P1 (sibling drain): Correct — matching on P2 (task_done ordering): Correct — P3 (test mechanism): Correct — the test previously enqueued the independent event after the grandchild. It now parks bus B's consumer on the global lock (bus B's run loop |
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
|
Addressed in 6ef3c3e: extracted a shared |
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="bubus/models.py">
<violation number="1" location="bubus/models.py:303">
P2: The lookup snapshot is frozen once, but the drain loop below still re-enumerates `list(EventBus.all_instances)` on every iteration (`for bus in list(EventBus.all_instances)`). A bus created mid-drain is iterated by the drain but is invisible to `_find_event_by_id`, which only scans `buses_snapshot`. Because the snapshot was taken before those buses existed, any of their queued events that belong to this waiting chain get `parent_candidate = None` during the descendant walk and are never drained, while unrelated events on them are scanned in the loop — so the drain's behavior silently depends on when the snapshot happened and contradicts the stated "snapshot once per drain" intent.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| # events across this set, and rebuilding it per hop would make | ||
| # the drain O(candidates × bus_count × chain_depth) per | ||
| # iteration (review feedback on #5509). | ||
| buses_snapshot = list(EventBus.all_instances) |
There was a problem hiding this comment.
P2: The lookup snapshot is frozen once, but the drain loop below still re-enumerates list(EventBus.all_instances) on every iteration (for bus in list(EventBus.all_instances)). A bus created mid-drain is iterated by the drain but is invisible to _find_event_by_id, which only scans buses_snapshot. Because the snapshot was taken before those buses existed, any of their queued events that belong to this waiting chain get parent_candidate = None during the descendant walk and are never drained, while unrelated events on them are scanned in the loop — so the drain's behavior silently depends on when the snapshot happened and contradicts the stated "snapshot once per drain" intent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At bubus/models.py, line 303:
<comment>The lookup snapshot is frozen once, but the drain loop below still re-enumerates `list(EventBus.all_instances)` on every iteration (`for bus in list(EventBus.all_instances)`). A bus created mid-drain is iterated by the drain but is invisible to `_find_event_by_id`, which only scans `buses_snapshot`. Because the snapshot was taken before those buses existed, any of their queued events that belong to this waiting chain get `parent_candidate = None` during the descendant walk and are never drained, while unrelated events on them are scanned in the loop — so the drain's behavior silently depends on when the snapshot happened and contradicts the stated "snapshot once per drain" intent.</comment>
<file context>
@@ -295,6 +295,19 @@ async def wait_for_handlers_to_complete_then_return_event():
+ # events across this set, and rebuilding it per hop would make
+ # the drain O(candidates × bus_count × chain_depth) per
+ # iteration (review feedback on #5509).
+ buses_snapshot = list(EventBus.all_instances)
+
+ def _find_event_by_id(event_id: str) -> BaseEvent[Any] | None:
</file context>
Summary
Fixes browser-use/browser-use#5509 — cross-loop contamination in the EventBus drain loop.
When a handler on bus A awaits a child event (e.g. via
await eventinside a handler context while holding the global lock), the__await__drain loop iterated over allEventBus.all_instancesand processed queued events from every bus — not just the waiting chain. With multiple parallel agent sessions (each with its own bus), an unrelated event queued on bus B during that window was processed inside bus A's handler context, stealing bus B's scheduling. With many parallel sessions this drives the capacity errors / agent run failures reported in the issue.Root cause
BaseEvent.__await__→wait_for_handlers_to_complete_then_return_event()inbubus/models.py:Every queued event on every bus was consumed, regardless of whether it belonged to the waiting chain.
Fix
{self} ∪ {parents}).sleep(0)yield). The underlying deque is indexed directly (same approach as the existing memory-usage check);task_done()pairs with theput()that enqueued the event.Tests
tests/test_issue5509_cross_loop.py: a bus A handler chain forwards its grandchild to bus B, while an unrelated event is queued on bus B first. Asserts the bus B event is not processed inside bus A's drain loop (fails before the fix withorder == ['a_drain_enter', 'b_handled', 'a_drain_exit'], passes after with the bus B event handled by bus B's own run loop afterwards).ruff checkandcodespellclean.Disclosure
This PR was developed with AI assistance (analysis and test-driven implementation), reviewed and verified by the author.