Skip to content

Don't drain other event loops' buses in BaseEvent.__await__ - #30

Open
syahra712 wants to merge 2 commits into
browser-use:mainfrom
syahra712:fix/cross-loop-await-drain
Open

syahra712 wants to merge 2 commits into
browser-use:mainfrom
syahra712:fix/cross-loop-await-drain

Conversation

@syahra712

@syahra712 syahra712 commented Aug 21, 2026

Copy link
Copy Markdown

Fixes browser-use/browser-use#5509

When a handler awaits an event, BaseEvent.__await__ enters a drain loop that
iterates EventBus.all_instances — every bus in the process — and runs their
queued handlers on the awaiting task's loop.

In an app with more than one event loop (e.g. several agent sessions running in
parallel, each with its own loop), this executes one bus's handlers on another
bus's loop. Those handlers hang, events get stuck in started, and they pile up
until the bus hits its 100-event capacity:

The drain loop should only touch buses that belong to the current running loop —
every other bus is already drained by its own _run_loop on its own loop.

Changes:

  • Track each bus's owning loop in EventBus._loop (set in _start).
  • Skip buses whose _loop isn't the current running loop in the __await__ drain.

Added tests/test_cross_loop_isolation.py: a bus on a second loop is kept busy
while a handler on the main loop awaits its event; the test asserts that event is
never run on the wrong loop. It fails on main and passes with the fix. Full
suite passes (139 tests).


Summary by cubic

Prevent cross-loop handler execution by making BaseEvent.__await__ drain only buses on the current, running event loop. Previously it drained all process-global buses and ran their handlers on the awaiting task’s loop; now it skips buses on other loops and stopped buses. Single-loop behavior is unchanged; multi-loop apps avoid stuck events and capacity overflows.

  • Track the owning loop in EventBus._loop (set in _start).
  • In the __await__ drain, skip buses whose _loop differs from the current loop or whose _is_running is False.
  • Add tests/test_cross_loop_isolation.py to assert a bus on another loop is never drained; make teardown unconditional to avoid leaked background threads.

Written for commit 40d6bbb. Summary will update on new commits.

Review in cubic

The drain loop in __await__ iterated EventBus.all_instances (process-global)
and ran each bus's queued handlers on the awaiting task's loop. In a multi-loop
app (parallel agent sessions, each on its own loop) that executed one bus's
handlers on another bus's loop, where they hung and accumulated until the bus
hit its 100-event capacity limit and dispatch() raised RuntimeError.

Track each bus's owning loop (set in _start) and skip buses whose loop is not
the current running loop; each bus's own _run_loop drains it on its own loop.

Closes browser-use/browser-use#5509.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 3 files

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="tests/test_cross_loop_isolation.py">

<violation number="1" location="tests/test_cross_loop_isolation.py:106">
P2: The cleanup block only runs when the test passes. Every assertion failure (and this regression test is meant to fail on the un-fixed branch, e.g. `assert ran_on.get('probe') != id(loop_a)`) raises before cleanup, leaving bus_b registered in `EventBus.all_instances` with its run loop still spinning and its background `loop_b` thread alive for the rest of the session, which can contaminate later tests. Move the body into `try:` and perform all stopping/shutdown in `finally:`.</violation>
</file>

<file name="bubus/models.py">

<violation number="1" location="bubus/models.py:317">
P2: After a bus stops with queued events, this check can still drain it because `_loop` remains set and shutdown leaves queued items. Require `bus._is_running` here or clear `_loop` during shutdown so stopped-bus handlers cannot run after `stop()`.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread tests/test_cross_loop_isolation.py Outdated
Comment thread bubus/models.py Outdated
- Drain guard also skips buses with _is_running False: a stopped bus can
  keep _loop set with events still queued, and nothing should run its
  handlers after stop().
- Move the regression test's assertions into try/ and all shutdown into
  finally/ so a failing run never leaks bus B's run loop or its thread.
shobhitsahani added a commit to shobhitsahani/browser-use that referenced this pull request Aug 22, 2026
…owser-use#5509)

## Summary

- Add `browser_use/bubus_compat.py`: an idempotent compatibility patch
  for the pinned `bubus==1.5.6` that stops `BaseEvent.__await__` from
  draining EventBuses owned by a different event loop.
- Record each bus's owning loop in `_start`, and in `__await__`'s
  drain/polling loop skip any bus whose `_loop` is not the current
  running loop (and any stopped bus). Every other bus is already
  drained by its own `_run_loop` on its own loop.
- Apply the patch at import time in `browser_use/__init__.py`, guarded
  by `try/except ImportError` so a bare `import browser_use` can never
  hard-fail on a missing or partially-installed bubus.
- Add `tests/ci/test_bubus_cross_loop_isolation.py` with a regression
  test that fails on unpatched bubus 1.5.6 and passes with the fix.

## Why

`bubus==1.5.6`'s `BaseEvent.__await__` iterates `EventBus.all_instances`
— the process-global WeakSet of every bus — and calls
`await bus.process_event(...)` on any bus with queued events, regardless
of which event loop each bus belongs to. With parallel agent sessions on
separate event loops (one EventBus per agent), one bus's handlers get
executed on another bus's loop, where they hang forever, pile up, and
eventually trip the 100-event capacity guard in `dispatch()`:

```text
RuntimeError: EventBus at capacity: 100 pending events (100 max).
Queue: 50, Processing: 50. Cannot accept new events until some complete.
```

The fix mirrors the canonical upstream change in browser-use/bubus#30
(not yet released), which makes the drain loop only touch buses started
on the current running loop. The patch auto-no-ops via
`hasattr(EventBus, '_loop')` once a fixed bubus is released.

## Reproduction

Before the fix, a standalone repro with a bus on a second event loop
showed the probe event run on the wrong (awaiting) loop:

```text
probe ran on loop A (BUG): True
RESULT: BUG REPRODUCED - cross-loop contamination
```

With the patch active the probe stays on its owning loop:

```text
probe ran on loop A (BUG): False
probe ran on loop B      : True
RESULT: OK - no cross-loop contamination
```

## Tests

- `uv run pytest tests/ci/test_bubus_cross_loop_isolation.py -q` — 2 passed
- `uv run pytest tests/ci/test_event_bus_resilience.py -q` — 5 passed
  (existing warm-resume/Restart event-bus flows remain green)
- Same-loop nested-await drain (the legit deadlock-avoidance path) still
  processes queued child events correctly.
- `ruff check` and `ruff format --check` pass on the new/changed source;
  `pyright` reports 0 errors / 0 warnings; `codespell` clean.
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.

bubus library EventBus capacity error — Cross-Loop Bug (service.py)

1 participant