fix(v3): guarantee event dispatch order under rapid Emit() bursts#5757
Open
DevLumuz wants to merge 2 commits into
Open
fix(v3): guarantee event dispatch order under rapid Emit() bursts#5757DevLumuz wants to merge 2 commits into
DevLumuz wants to merge 2 commits into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughEvent dispatch now uses buffered queues and persistent workers, with concurrency-safe test updates. On Linux GTK3, JavaScript execution now waits for WebKit completion through a new synchronous C helper. ChangesQueue-based Event Dispatch
Linux Synchronous JavaScript Execution
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant EventProcessor
participant ListenerWorker
participant WindowWorker
Caller->>EventProcessor: Emit(event)
EventProcessor->>EventProcessor: enqueue to listenerQueue
EventProcessor->>EventProcessor: enqueue to windowQueue
ListenerWorker->>ListenerWorker: dispatchEventToListeners(event)
WindowWorker->>WindowWorker: dispatchEventToWindows(event)
sequenceDiagram
participant linuxWebviewWindow
participant execute_js_sync
participant WebKit
participant GMainContext
linuxWebviewWindow->>execute_js_sync: C.execute_js_sync(js, world)
execute_js_sync->>WebKit: webkit_web_view_evaluate_javascript(...)
WebKit->>execute_js_sync: on_evaluate_javascript_finish()
execute_js_sync->>GMainContext: iterate until completion flag set
execute_js_sync-->>linuxWebviewWindow: return after completion
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain modules listed in go.work or their selected dependencies" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
v3/pkg/application/events.go (1)
122-155: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a shutdown path for EventProcessor
NewWailsEventProcessorstarts two goroutines that block forever on unclosed queues. That is fine for the app singleton, but short-lived processors in tests and benchmarks—likeBenchmarkOnMultipleEventinsideb.Loop()—keep accumulating workers and skew benchmark results. AddClose/Stopand stop the dispatch loops with a done channel or by closing the queues.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@v3/pkg/application/events.go` around lines 122 - 155, Add a shutdown path to EventProcessor so the goroutines started by NewWailsEventProcessor do not leak for short-lived uses. Introduce a Close/Stop method on EventProcessor, track a done signal or close listenerQueue/windowQueue safely, and make runListenerDispatch and runWindowDispatch exit when shutdown is requested. Update NewWailsEventProcessor and the dispatch loops to use the new lifecycle control so tests and benchmarks like BenchmarkOnMultipleEvent can stop workers cleanly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@v3/pkg/application/events.go`:
- Around line 200-201: `EventManager.Emit` is currently blocked by synchronous
sends to both `listenerQueue` and `windowQueue`, so a slow window delivery path
can stall callers. Update the `Emit` flow in `EventManager` to avoid blocking on
`windowQueue` backpressure by using a non-blocking enqueue strategy with
overflow handling, such as logging and dropping or otherwise degrading safely,
while keeping `listenerQueue` behavior consistent with the intended delivery
model. Refer to `Emit`, `listenerQueue`, `windowQueue`, and the
`EventIPCTransport.DispatchWailsEvent`/`WebviewWindow.DispatchWailsEvent` path
when making the change.
In `@v3/pkg/application/linux_cgo_gtk3.go`:
- Around line 124-139: `execute_js_sync` can still allow out-of-order `execJS`
completion because it blocks with a nested main loop while another
`webkit_web_view_evaluate_javascript` is already in flight. Update the
`execute_js_sync`/`on_evaluate_javascript_finish` flow so JS executions are
serialized through a single queue or by chaining the next request from the
previous callback. Make sure only one `WebKitWebView` evaluation is active at a
time, and keep the existing synchronous behavior at the `execute_js_sync` call
site.
---
Nitpick comments:
In `@v3/pkg/application/events.go`:
- Around line 122-155: Add a shutdown path to EventProcessor so the goroutines
started by NewWailsEventProcessor do not leak for short-lived uses. Introduce a
Close/Stop method on EventProcessor, track a done signal or close
listenerQueue/windowQueue safely, and make runListenerDispatch and
runWindowDispatch exit when shutdown is requested. Update NewWailsEventProcessor
and the dispatch loops to use the new lifecycle control so tests and benchmarks
like BenchmarkOnMultipleEvent can stop workers cleanly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7346d191-d362-4292-a4a9-bec0db30deff
📒 Files selected for processing (3)
v3/pkg/application/events.gov3/pkg/application/events_test.gov3/pkg/application/linux_cgo_gtk3.go
Two related fixes for Wails events reaching the page out of order when
Emit() is called rapidly in succession (e.g. streaming many small events
back-to-back):
1. EventProcessor.Emit spawned two bare goroutines per call for dispatch
(listeners, windows), with no ordering guarantee between goroutines from
successive calls. Replaced with one buffered, ordered worker per target —
Emit() stays non-blocking, dispatch is now strict FIFO. Also bounds
in-flight dispatch (dispatchQueueSize), unlike the old unbounded fan-out.
2. linux/gtk3 execJS passed callback=NULL to
webkit_web_view_evaluate_javascript ("fire and forget"): WebKit confirms
the request was accepted, not that it finished executing, so nothing
guaranteed successive evaluate_javascript calls ran in the order they
were dispatched. execJS now waits for the real completion callback, via
the same nested-main-loop technique already used by
clipboard_get_text_sync (linux_cgo.c).
Verified with an isolated repro (500 Emit() calls in a tight loop, no AI/LLM
or app-specific code involved): before, 478/500 (95.6%) arrived out of
order in the page. After, 0/500. Also faster to issue: 2.16ms -> 244µs total
for the 500 calls, since Emit() no longer spawns two goroutines per call.
go test -race ./pkg/application/... passes clean (also fixed two
pre-existing latent races in events_test.go's mock that -race now catches
reliably against the new, more deterministic dispatch timing).
DevLumuz
force-pushed
the
fix-event-dispatch-order
branch
from
July 6, 2026 00:49
5f00091 to
6825ea2
Compare
|
That’s awesome! Right now I’m outputting SSE data to the frontend and relying on sleep delays to maintain the order. With this solution, everything will work perfectly without any issues. |
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.
Problem
Events emitted rapidly in succession (e.g. a Go backend streaming many small
events back-to-back) can reach the frontend out of order, or with visible
interleaving. Two independent causes:
EventProcessor.Emit(v3/pkg/application/events.go) spawns two baregoroutines per call for dispatch (one for Go listeners, one for windows).
Go gives no ordering guarantee between separately spawned goroutines, so
two events emitted back-to-back can have their window-dispatch (and
therefore their
ExecJScall) run in either order.linux/gtk3
execJS(v3/pkg/application/linux_cgo_gtk3.go) callswebkit_web_view_evaluate_javascriptwithcallback=NULL("fire andforget"). WebKit confirms the request was accepted, not that it
finished executing — WebKit evaluates scripts asynchronously via its
WebProcess, and without a completion callback nothing guarantees that
rapid, back-to-back
evaluate_javascriptcalls execute in the same orderthey were dispatched.
Fix
Replaced the two bare goroutines in
Emit()with one buffered, orderedworker per dispatch target (listeners, windows).
Emit()staysnon-blocking under normal load (buffer of 4096, well above realistic
bursts) — dispatch is now strict FIFO per target, and also bounded,
unlike the previous unbounded goroutine-per-
Emit()fan-out.execJSnow waits for the real WebKit completion callback beforereturning, using the same nested main-loop-iteration technique already
used by
clipboard_get_text_sync(linux_cgo.c) to make an asyncGLib/GTK call appear synchronous from the calling C code — blocking on a
condition variable instead would freeze the very loop that has to keep
running for the completion callback to ever be delivered.
Also fixed two pre-existing latent races in
events_test.go's mock notifierand counter that
-racenow catches reliably against the new, moredeterministic dispatch timing (they were always technically possible, just
rarely triggered by the old bare-goroutine timing).
Verification
Isolated repro: 500
Emit()calls in a tight loop, no app-specific codeinvolved, checking arrival order in the page.
Emit()callsEmit()no longer spawns two goroutines per call)go test -race ./pkg/application/...passes clean.Summary by CodeRabbit