Conversation
EventEmitter kept handlers in a set, so emit walked them in hash order. That is stable for the life of a process and arbitrary between runs, so a handler that reads a field a peer handler writes during the same emit is correct on one machine and wrong on another, consistently rather than intermittently. livekit-agents hits this: AgentActivity._on_metrics_collected stamps speech_id onto the emitted object before re-emitting, so a second subscriber to the same plugin event sees the value or None depending on which handler the set yields first. The reporter measured the pre-stamp state in 7 of 12 fresh processes. A dict keyed by the handler keeps insertion order and the idempotent add a set gave, so registering twice still dispatches once and does not reorder. The ordering tests pin hashes rather than registering a few handlers and hoping a set shuffles them, which would only fail about one run in six.
ubmids
requested review from
cloudwebrtc,
lukasIO and
xianshijing-lk
as code owners
September 19, 2026 15:06
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Member
|
fixed by #802 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #778.
EventEmitterstored handlers in aset, soemitwalked them in hash order. That order is stable for the life of a process and arbitrary between runs, which is an awkward shape: a handler that reads a field a peer handler writes during the same emit is consistently right on one machine and consistently wrong on another, rather than flaky on both.The reported case is in livekit-agents, where
AgentActivity._on_metrics_collectedstampsspeech_idonto the emitted object before re-emitting it, so a second subscriber to the same plugin'smetrics_collectedsees the stamped value orNonedepending on which handler the set happens to yield first. #778 measured the pre-stamp state in 7 of 12 fresh processes._eventsis now a dict keyed by the handler, which is what #778 suggested. Insertion order is preserved andsetdefaultkeeps the idempotent add a set gave, so registering the same handler twice still dispatches once and does not move it to the back.Worth flagging up front:
emitstill walks a copy, so anoffinside a handler takes effect from the next emit and not the one that removed it.oncedepends on that, and there is a test pinning it, because an ordered container is an easy place to accidentally start iterating the live collectionOn the tests: they pin handler hashes rather than registering a few callbacks and hoping a
setreorders them. With three handlers that would pass on the broken implementation about one run in six, and a test that only usually fails is not much of a regression test. Four of the six fail onmain; the other two cover the dedup and copy-during-emit behaviour, which were already correct and are there to stop the change quietly breaking them.Checked with
pytest livekit-rtc/tests(63 passed, 14 skipped),ruff check,ruff format --checkandmypy.Written with AI assistance; I have gone through every line and can speak to it.