fix(v3): keep oversized event payloads out of evaluateJavaScript - #5930
Merged
Conversation
Go->JS events are delivered by splicing the event JSON into a JavaScript source string and evaluating it in the webview. Above a size threshold, WebKit switches from inline IPC message data to an out-of-line shared-memory transfer, and those regions are retained for as long as the app keeps emitting. Measured with v3/tests/event-performance at 100 events/sec of 1MB: macOS 26.4.1 host process -> 11,629 MB Ubuntu 26.04 web process -> 6,262 MB The two ports differ in which process holds the memory, and in where the threshold sits (macOS 8-16KB, WebKitGTK 64-128KB), but the mechanism is the same. On macOS vmmap attributes all of it to "owned unmapped", at exactly one region per oversized eval. Below the threshold there is no retention at all: 1.19M events at 64B left the content process at its own floor, so this is a function of per-event payload size, not call rate. Events whose marshalled JSON exceeds maxInlineEventPayload are now parked host-side under a random 128-bit id and fetched by the webview over the asset server, which delivers via a URL scheme task and never touches the eval IPC path. Small events keep the existing inline path unchanged. macOS size-1MB 11,629 MB -> 65.8 MB (177x) macOS iso-64KB 1,004 MB -> 56.5 MB (18x) Linux size-1MB 6,262 MB -> 225 MB (28x) vmmap confirms "owned unmapped" disappears from the region table entirely and host footprint holds flat at 33.8 MB across the run. Ordering is preserved: once a window has sent one event by reference it routes all subsequent events through the same JS promise chain, so a small event cannot overtake a large one still being fetched. Verified with an interleave scenario alternating 1KB and 64KB payloads - consecutive sequence numbers travelling by different mechanisms - at 0 reorders and 0 drops on both platforms. The payload store is one-shot on read, bound to the dispatching window via the existing x-wails-window-id header, TTL-reaped at 30s, capped at 64MB with fallback to inline on overflow, and released on window destroy. No public API change, and no runtime JS change: all three delivery templates call the same window._wails.dispatchWailsEvent entry point and keep the existing guard for the runtime not yet being mounted. Co-authored-by: superDingda <89772770+superDingda@users.noreply.github.com>
A standalone Wails app under v3/tests/event-performance that measures the
Go->JS event transport and produces a repeatable before/after number.
It drives events straight at WebviewWindow.DispatchWailsEvent, the exact
entry point that performs the eval, so the measurement is of the transport
rather than the runtime's listener plumbing or the app-level fanout. The
page intercepts window._wails.dispatchWailsEvent by pre-creating the
_wails object with an accessor, which survives whichever runtime module
mounts first.
Samples both the host process and the web content process every 250ms.
Watching only one of them misses the bug: on macOS the retention shows up
in the host, on Linux in the web process.
sampler_darwin.go proc_pid_rusage ri_phys_footprint; content process
located by diffing the pid set across window creation
(an empty diff aborts rather than summing every
WebContent on the machine)
sampler_linux.go Pss via smaps_rollup; WebKitWebProcess located via
/proc/*/cmdline, since comm truncates at 15 chars and
the name is 16
sampler_other.go stubs, so timing and ordering still run elsewhere
Verdicts grade on floor rise - the minimum of the window's second half
minus the minimum of its first - not on a regression slope. Memory under
GC is a sawtooth, so a slope mostly reports which phase the window opened
in, and it produced false leak labels on the first pass. A process that is
genuinely leaking cannot return to its earlier floor. Go MemStats are
recorded alongside, so native growth can be told apart from Go heap.
Scenarios:
idle baseline drift with a webview open and no events
rate-* 10..5000 ev/s at 64B - isolates call count
size-* 1KB..1MB at 100 ev/s - isolates payload size
iso-* ~4MB/s held constant, 1KB..64KB - locates the threshold
iso2-* ~32MB/s held constant, 32KB..1MB - for higher knees
interleave alternating 1KB/64KB - ordering across delivery modes
mixedsource goroutine + main-thread emitters on one sequence
burst mean 1000 ev/s in bursts of 20
pathological 10 ev/s at 8MB
Outputs a CSV per scenario, summary.json, and REPORT.md.
Known result: mixedsource reports ~4.4% inversions on master. That is a
pre-existing hazard - dispatchOnMainThread runs inline when already on the
main thread, so a main-thread emit overtakes a queued goroutine emit - and
is unrelated to event payload size. Recorded here as a regression test for
whoever fixes it.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR adds bounded host-side storage for oversized events, one-shot window-bound retrieval, ordered JavaScript delivery, and a cross-platform event-performance harness with scenario execution, memory sampling, and report generation. ChangesEvent delivery and performance measurement
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant GoHarness
participant WebviewWindow
participant eventPayloadStore
participant JavaScriptRuntime
participant ReportEndpoint
participant ReportGenerator
GoHarness->>WebviewWindow: Emit configured event scenarios
WebviewWindow->>eventPayloadStore: Store oversized event payload
WebviewWindow->>JavaScriptRuntime: Deliver inline or fetch-reference event
JavaScriptRuntime->>eventPayloadStore: Fetch referenced payload
JavaScriptRuntime->>ReportEndpoint: Send delivery and frame metrics
GoHarness->>ReportGenerator: Provide emission, memory, and process samples
ReportEndpoint-->>ReportGenerator: Provide JavaScript reports
ReportGenerator-->>GoHarness: Write verdicts and scenario reports
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![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: 5
🧹 Nitpick comments (6)
v3/tests/event-performance/sampler_other.go (1)
5-8: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUpdate the stale comment.
The build tag excludes darwin and linux, but the comment states "unsupported off darwin". Linux sampling now exists in
sampler_linux.go.✏️ Proposed change
-// Memory metrics are unsupported off darwin, but the timing and ordering half -// of the harness still runs so engines can be compared later. +// Memory metrics are unsupported outside darwin and linux, but the timing and +// ordering half of the harness still runs so engines can be compared later.🤖 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/tests/event-performance/sampler_other.go` around lines 5 - 8, Update the comment above samplerSupported in sampler_other.go to reflect that memory metrics are unsupported only on platforms other than Darwin and Linux, while timing and ordering comparisons still run there.v3/tests/event-performance/scenarios.go (2)
170-182: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace
itoawithstrconv.Itoa.The hand-written
itoaduplicates the standard library and returns an empty string for negative input.strconv.Itoais correct for all values.♻️ Proposed replacement
-func itoa(i int) string { - if i == 0 { - return "0" - } - var b [20]byte - p := len(b) - for i > 0 { - p-- - b[p] = byte('0' + i%10) - i /= 10 - } - return string(b[p:]) -}Then use
strconv.Itoa(r)at Line 60 and import"strconv".🤖 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/tests/event-performance/scenarios.go` around lines 170 - 182, Remove the custom itoa function and use strconv.Itoa for integer-to-string conversion, including replacing the call that converts r. Add the strconv import and preserve the existing output for all integer values, including negatives.
152-168: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReport unknown scenario names in
-only.
selectedScenariosdrops names that do not match a scenario. If the user mistypes a name, the harness runs zero scenarios and still writes an empty report. Return an error or log the unmatched names.♻️ Proposed change to surface unmatched names
var out []Scenario for _, s := range all { if want[s.Name] { out = append(out, s) + delete(want, s.Name) } } + for n := range want { + log.Printf("unknown scenario name in -only: %q", n) + } return out }Add
"log"to the imports.🤖 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/tests/event-performance/scenarios.go` around lines 152 - 168, Update selectedScenarios to detect names from only that do not match any Scenario in allScenarios, and surface each unmatched name through the established error or logging path instead of silently returning no scenarios. Preserve matching and empty-input behavior.v3/tests/event-performance/main.go (1)
464-477: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the platform process name in the error message.
discoverWebContentis shared by all platforms, but the error text namescom.apple.WebKit.WebContent. On Linux the harness looks forWebKitWebProcess. Both samplers already exportwebContentProcName, so use that constant.♻️ Proposed fix
- return fmt.Errorf("no new com.apple.WebKit.WebContent process appeared after window creation "+ - "(%d existed before launch); cannot attribute one to this app", len(h.basePids)) + return fmt.Errorf("no new %s process appeared after window creation "+ + "(%d existed before launch); cannot attribute one to this app", + webContentProcName, len(h.basePids))
sampler_other.gomust then also declarewebContentProcName.Based on learnings that platform-specific behavior in shared files should be controlled at compile time rather than hardcoded for one OS.
🤖 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/tests/event-performance/main.go` around lines 464 - 477, Update discoverWebContent to use the shared platform-specific webContentProcName constant in its error message instead of hardcoding com.apple.WebKit.WebContent. Ensure sampler_other.go also declares webContentProcName with the Linux process name WebKitWebProcess, matching the existing declaration used by the other sampler.Source: Learnings
v3/tests/event-performance/report.go (2)
356-375: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGate the macOS-only environment probes.
sw_vers,machdep.cpu.brand_string, and the WebKitInfo.plistread exist on macOS only. On Linux the harness spawns three processes per report and records nothing. Split this function per build tag, or read/proc/cpuinfoand/etc/os-releaseon Linux.Based on learnings that platform-specific behavior should be controlled via build tags rather than executed unconditionally in shared files.
🤖 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/tests/event-performance/report.go` around lines 356 - 375, Update environment() so macOS-specific probes (sw_vers, machdep.cpu.brand_string, and the WebKit Info.plist read) are compiled only for macOS via build-tagged platform-specific implementations. Keep the shared environment values and git commit collection available across platforms, and provide the Linux implementation using appropriate Linux system sources such as /proc/cpuinfo and /etc/os-release.Source: Learnings
72-82: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCheck
gofmtchanges for these event-performance files.
gofmt -llistsv3/tests/event-performance/report.goandv3/tests/event-performance/scenarios.goas requiring formatting. Update these files so repository formatting checks do not fail.🤖 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/tests/event-performance/report.go` around lines 72 - 82, Run gofmt on report.go and scenarios.go, applying the formatter’s alignment and spacing changes throughout both event-performance files. Preserve all existing behavior and values; only update formatting so gofmt -l no longer reports either file.
🤖 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/application.go`:
- Line 75: In New, immediately after initializing result.eventPayloads with
newEventPayloadStore(), register result.eventPayloads.close through
result.OnShutdown so the reap goroutine is stopped during application shutdown.
In `@v3/pkg/application/event_payload_store.go`:
- Around line 127-139: Update the cleanup loop in the event payload store to
sweep more frequently than the 30-second TTL, and change its expiration
condition to treat entries whose deadline equals now as expired using
item.created.Add(eventPayloadTTL).After(now). Preserve the existing deletion and
byte-accounting behavior.
In `@v3/pkg/application/webview_window.go`:
- Around line 1420-1428: Serialize event mode selection, payload storage, and
JavaScript enqueueing in the window event-dispatch path using a per-window mutex
or shared ordered dispatcher. Update the logic around eventRefMode and ExecJS so
concurrent inline and reference-mode events cannot interleave during the
transition, preserving event order for all subsequent events.
In `@v3/tests/event-performance/report.go`:
- Around line 174-176: Update the ByteRatePerSec calculation in the report
generation logic to account for both Payload and AltPayloadBytes, using the
effective or average payload size for interleave scenarios. Preserve the
existing Sent and secs scaling so the reported rate reflects all transmitted
bytes.
In `@v3/tests/event-performance/sampler_darwin.go`:
- Around line 12-14: Update the Darwin process-count handling in webContentPids
to use the value returned by hp_listpids directly as the pid count; remove the
extra division by sizeof(C.int) while preserving the existing scan and
discoverWebContent behavior.
---
Nitpick comments:
In `@v3/tests/event-performance/main.go`:
- Around line 464-477: Update discoverWebContent to use the shared
platform-specific webContentProcName constant in its error message instead of
hardcoding com.apple.WebKit.WebContent. Ensure sampler_other.go also declares
webContentProcName with the Linux process name WebKitWebProcess, matching the
existing declaration used by the other sampler.
In `@v3/tests/event-performance/report.go`:
- Around line 356-375: Update environment() so macOS-specific probes (sw_vers,
machdep.cpu.brand_string, and the WebKit Info.plist read) are compiled only for
macOS via build-tagged platform-specific implementations. Keep the shared
environment values and git commit collection available across platforms, and
provide the Linux implementation using appropriate Linux system sources such as
/proc/cpuinfo and /etc/os-release.
- Around line 72-82: Run gofmt on report.go and scenarios.go, applying the
formatter’s alignment and spacing changes throughout both event-performance
files. Preserve all existing behavior and values; only update formatting so
gofmt -l no longer reports either file.
In `@v3/tests/event-performance/sampler_other.go`:
- Around line 5-8: Update the comment above samplerSupported in sampler_other.go
to reflect that memory metrics are unsupported only on platforms other than
Darwin and Linux, while timing and ordering comparisons still run there.
In `@v3/tests/event-performance/scenarios.go`:
- Around line 170-182: Remove the custom itoa function and use strconv.Itoa for
integer-to-string conversion, including replacing the call that converts r. Add
the strconv import and preserve the existing output for all integer values,
including negatives.
- Around line 152-168: Update selectedScenarios to detect names from only that
do not match any Scenario in allScenarios, and surface each unmatched name
through the established error or logging path instead of silently returning no
scenarios. Preserve matching and empty-input behavior.
🪄 Autofix
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 Plus
Run ID: 2b5634a5-edad-4e6c-ab36-2fd846f0c6be
📒 Files selected for processing (10)
v3/pkg/application/application.gov3/pkg/application/event_payload_store.gov3/pkg/application/webview_window.gov3/tests/event-performance/assets/index.htmlv3/tests/event-performance/main.gov3/tests/event-performance/report.gov3/tests/event-performance/sampler_darwin.gov3/tests/event-performance/sampler_linux.gov3/tests/event-performance/sampler_other.gov3/tests/event-performance/scenarios.go
Contributor
There was a problem hiding this comment.
Pull request overview
This PR changes Wails v3 Go→JS event delivery to avoid WebKit’s out-of-line shared-memory retention when oversized event JSON is spliced into evaluateJavaScript source. Large event payloads are now parked host-side and fetched by the webview via the asset server, while small events keep the existing inline eval path; a new v3/tests/event-performance harness is added to measure memory/ordering/timing.
Changes:
- Route oversized event payloads through a host-side payload store + asset-server fetch (small payloads remain inline).
- Preserve in-order delivery by chaining dispatch through a per-window JS promise once any by-reference event is used.
- Add a cross-platform event dispatch performance harness (
v3/tests/event-performance) with memory samplers for darwin/linux.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| v3/pkg/application/webview_window.go | Adds inline/chained/ref delivery templates and switches oversized events to by-reference fetch. |
| v3/pkg/application/event_payload_store.go | Introduces TTL/capped store for oversized event payloads keyed by random IDs. |
| v3/pkg/application/application.go | Hooks /wails/eventpayload/… into the asset server to serve one-shot payload fetches; stores the payload store on App. |
| v3/tests/event-performance/main.go | Adds the event dispatch benchmark harness app and scenario runner. |
| v3/tests/event-performance/report.go | Computes/exports CSV + summary.json + markdown report with floor-rise grading. |
| v3/tests/event-performance/scenarios.go | Defines the scenario matrix (rate sweep, size sweep, iso-byte-rate, ordering probes). |
| v3/tests/event-performance/assets/index.html | Adds a runtime interception page to measure drops/reorders/latency and report to Go. |
| v3/tests/event-performance/sampler_darwin.go | Adds macOS WebContent PID discovery + footprint sampling via libproc. |
| v3/tests/event-performance/sampler_linux.go | Adds Linux WebKitWebProcess PID discovery + PSS/RSS sampling via /proc. |
| v3/tests/event-performance/sampler_other.go | Adds a stub sampler for unsupported platforms (timing/ordering only). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
74
to
76
| result.customEventProcessor = NewWailsEventProcessor(result.Event.dispatch) | ||
| result.eventPayloads = newEventPayloadStore() | ||
|
|
Member
Author
There was a problem hiding this comment.
Fixed — OnShutdown(eventPayloads.close) is registered where the store is created, so the reaper doesn't outlive App.cleanup(). A follow-up review also found that close racing put could strand a payload and that concurrent close could panic; both are handled now with a mutex-guarded closed flag and a sync.Once. (5ed70d3, 283141790)
Three findings from automated review: - CodeQL flagged the ParseUint -> uint conversion for the window id header as truncating on 32-bit builds. Parse at strconv.IntSize so the value cannot exceed uint width. - The store owns a reaper goroutine, but nothing stopped it: window close only calls dropWindow, and close was never wired up. Register it via OnShutdown alongside store initialisation. - The reaper swept once per TTL, so an entry created just after a sweep survived until the following one - holding it for nearly 60s against a documented 30s bound. Sweep at TTL/4 and treat an entry as expired when its deadline is reached rather than strictly passed.
WebView2 has no single content process: an app gets a browser process plus renderer, GPU and utility children, all named msedgewebview2.exe, and a dev box typically has a dozen belonging to other apps already running. The existing pre-launch pid diff handles that - every pid appearing after our window is created is ours - so the sampler only has to enumerate and measure. Memory is reported as PrivateUsage from GetProcessMemoryInfo, the closest Windows analogue to phys_footprint: private committed bytes, excluding the mapped images every WebView2 child shares, which would otherwise be counted once per process.
Windows 11 / WebView2 151 does not have the retention: the host process holds flat at 65-77 MB where macOS reached 11.6 GB, and the content process sawtooths under V8 GC identically in patched and unpatched builds. So 8192 is correct on all three engines and conservative off macOS rather than unverified.
leaanthony
force-pushed
the
fix/oversized-event-payloads
branch
from
August 9, 2026 13:09
5ed70d3 to
b086266
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/event_payload_store.go`:
- Around line 159-165: Add mutex-protected shutdown state to eventPayloadStore
and update close and put so shutdown and payload insertion are serialized: close
must mark the store closed before stopping the janitor, and put must return
false without storing anything once shutdown has started. Replace the
select-and-close logic in close with a dedicated sync.Once to make concurrent
shutdown calls safe, while preserving normal insertion before shutdown.
In `@v3/tests/event-performance/sampler_windows.go`:
- Around line 49-59: Update webContentPids to return and propagate snapshot and
Process32First errors instead of returning nil as an empty process list. At its
callers in main.go, treat enumeration failure as invalid memory sampling or stop
the Windows run, preventing failed pre-launch enumeration from being used as a
valid baseline.
🪄 Autofix
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 Plus
Run ID: f41dfbf2-3fda-477b-90b3-6edc312c2467
📒 Files selected for processing (3)
v3/pkg/application/event_payload_store.gov3/tests/event-performance/sampler_other.gov3/tests/event-performance/sampler_windows.go
Ordering race (CodeRabbit): a small event could read eventRefMode as false while a large event sat between store.put and the flag being set, then dispatch inline and overtake it. Fixed, but not with a per-window mutex as suggested - that would have to be held across the ExecJS main-thread hop, which is the same shape as the deadlock documented in transport_event_ipc.go. Instead the decision moves into JavaScript: evals execute serially on the UI thread in enqueue order, so testing w.__eq there is race-free by construction and needs no Go-side state. eventRefMode is gone. Unterminated promise chain (Copilot): the chained template had no catch, so one throwing listener would leave w.__eq permanently rejected and silently stop all later events for that window. Both templates now terminate with catch. close/put race (CodeRabbit): put could store a payload after close, leaving it unreapable, and concurrent close could panic on a double channel close. Added a mutex-guarded closed flag that makes put refuse, and a dedicated sync.Once around the channel close. Endpoint hardening (Copilot): restrict to GET/HEAD and validate the id is exactly 32 hex characters before touching the map. proc_listallpids returns a PID COUNT, not a byte count (CodeRabbit and Copilot, independently). Verified empirically on macOS: 907 returned against 907 pids present. Dividing by sizeof(int) meant scanning a quarter of the process table, which could have missed our own content process and produced a spurious "no new WebContent process" abort. It found the pid anyway in every run so far, by luck. Enumeration failures no longer look like an empty process list (CodeRabbit): webContentPids returns an error on all platforms, a failed pre-launch enumeration is now fatal rather than silently classifying every pre-existing content process as ours, and discovery propagates the error. ByteRatePerSec ignored AltPayloadBytes (CodeRabbit), understating the interleave scenario's byte rate ~32x in the column that carries the attribution signal. Now averages the two sizes. Verified after the changes: 224,115 events at 4980/s with 0 drops and 0 reorders, emit p50 42us (unchanged); interleave 0 reorders across mixed inline and by-reference delivery; size-1MB host peak 54 MB.
Two things. My eventPayloads field broke the alignment of the surrounding struct fields, which is mine to fix. The rest is a pre-existing mis-indent of the MCP server block that gofmt corrects; whitespace only, no semantic change.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/tests/event-performance/sampler_other.go`:
- Around line 7-8: Update the comment above the fallback in the sampler harness
to describe platforms without a sampler, replacing the Darwin-specific “off
darwin” wording. Preserve the existing explanation that timing and ordering
still run for engine comparisons.
🪄 Autofix
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 Plus
Run ID: f218e116-51ca-44b2-b2e4-d7d4f7ac2ece
📒 Files selected for processing (10)
v3/pkg/application/application.gov3/pkg/application/event_payload_store.gov3/pkg/application/webview_window.gov3/tests/event-performance/main.gov3/tests/event-performance/report.gov3/tests/event-performance/sampler_darwin.gov3/tests/event-performance/sampler_linux.gov3/tests/event-performance/sampler_other.gov3/tests/event-performance/sampler_windows.gov3/tests/event-performance/scenarios.go
🚧 Files skipped from review as they are similar to previous changes (6)
- v3/tests/event-performance/sampler_linux.go
- v3/tests/event-performance/sampler_darwin.go
- v3/tests/event-performance/scenarios.go
- v3/tests/event-performance/main.go
- v3/pkg/application/event_payload_store.go
- v3/pkg/application/application.go
16 tasks
It said metrics are unsupported off darwin, which stopped being true once linux and windows samplers were added. It is the fallback for platforms that have no sampler, not for everything that is not darwin.
leaanthony
added a commit
that referenced
this pull request
Aug 9, 2026
Three problems in the asset server, found while investigating Go→JS event dispatch (#5930) and independent of it. contentTypeSniffer.Flush delegated to the wrapped writer without completing itself. Until 512 bytes have been seen the sniffer holds the body back to detect a Content-Type, so a flush before that point sent nothing and reported success — every streaming format silently delivered nothing. Flush now completes first, and complete advances the prefix only by the bytes the writer accepted, recording any error so it surfaces from the next Write or complete since http.Flusher cannot report one. The platform response writers did not implement http.Flusher, so http.ResponseController(w).Flush() could never reach them. macOS, iOS and Linux now declare a documented no-op — Write already pushes to the URL scheme task or into the pipe WebKitGTK reads. Windows deliberately does not: its Write accumulates into an in-memory buffer handed to WebView2 only in Finish, so a no-op there would claim a capability that does not exist. Leaving it unimplemented keeps ResponseController.Flush returning ErrNotSupported, which is true — and means streaming cannot work on Windows today regardless of WebView2. dispatchWorkers is declared and read but never assigned, which reads like an oversight and is not: it must stay 0. Any positive value switches to a fixed worker pool where a long-lived request holds its worker for its lifetime, starving later requests — including the page's own assets — and presenting as an unexplained startup hang. Documented rather than removed. Eight tests cover the sniffer; three fail without the fix.
leaanthony
pushed a commit
that referenced
this pull request
Aug 9, 2026
…ed event payloads out of evaluateJavaScript
16 tasks
leaanthony
added a commit
that referenced
this pull request
Aug 10, 2026
App.dispatchOnMainThread runs work inline when the caller is already on the UI thread, so an event emitted from the UI thread executed its eval immediately while an event emitted moments earlier from a goroutine was still queued — and the later event arrived first. DispatchWailsEvent now appends to a per-window queue and a single drain on the UI thread empties it in order. Appending under a mutex makes the queue the one ordering authority, whichever thread each event came from. A lock around the dispatch instead would deadlock: it would have to be held across the ExecJS main-thread hop, so a UI-thread emit would block on a lock whose holder is waiting for the UI thread. The queue avoids that because the bound applies to goroutine emitters only — the UI thread is the drainer and is never made to wait. Capacity 64, chosen by measurement. Ordering held and 5000 ev/s was sustained at every size tried, so only tail latency moved, rising with depth (179us p99 at 16, 427us at 1024). Emit also stopped blocking on a main-thread round trip, so emit p50 at 5000 ev/s fell from ~45us to 1-2us. Nothing is dropped when the queue is full: goroutine emitters block until a slot frees, the UI thread appends past the bound and drains inline, and a window destroyed while an emitter waits releases it. Note the bound limits this queue rather than total in-flight memory, since the mailbox added in #5851 is unbounded and sits upstream. Verified on macOS, Ubuntu 26.04 (both the default GTK4 backend and legacy -tags gtk3) and Windows 11 (WebView2): 0 inversions across mixed-source, interleaved-size and 5000 ev/s runs. Six unit tests need no GUI so they run in CI; the headline one drives DispatchWailsEvent and fails without the queue. Also corrects the harness scenario that first reported this, which took its sequence number outside the InvokeAsync callback and so measured its own race. The ~4.4% figure quoted in #5930 is retracted. Prior art: @DevLumuz raised event ordering first in #5757 and is credited on #5851 for the layer that fix addressed. The GTK3 concern from that PR was measured here at 186,047 events with 0 reorders and is not reproduced.
27 tasks
6 tasks
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
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.
Description
Go→JS events are delivered by splicing the event JSON into a JavaScript source string and evaluating it in the webview. Above a size threshold WebKit switches from inline IPC message data to an out-of-line shared-memory transfer, and those regions are retained for as long as the app keeps emitting.
At 100 events/sec of 1 MB payloads:
The two WebKit ports mirror each other in which process holds the memory — watching only one of them shows nothing. Chromium doesn't have the failure mode at all.
Fixes #4587
It's payload size, not call rate
A step function of per-event payload size, not of how many events you send:
Below the knee there is no retention at all — 1,191,895 events at 64 B (240 s at 5000 ev/s) left the content process at its own floor, ending lower than it started. macOS flips between 8–16 KB; WebKitGTK between 64–128 KB.
Mechanism
vmmapduring a leaking run — every mapped region flat or shrinking, all growth inowned unmapped(memory the process owns on the ledger but has unmapped: the sender's side of a shared-memory handoff):5,758 new regions for 5,760 events — one retained region per oversized eval.
Ruled out by experiment: an undelivered IPC backlog (max lag 26 events against 11.5 GB), a missing
@autoreleasepool(patched it; no change), and Wails' ownCString/NSString(removing only the eval call while keeping both holds the host at 53 MB vs 11,675 MB). Go heap stays flat at 24 MB throughout.The change
Events whose marshalled JSON exceeds
maxInlineEventPayload(8192) are parked host-side under a random 128-bit id and fetched by the webview over the asset server, which delivers via a URL scheme task and never touches the eval IPC path. Small events keep the existing inline path byte-for-byte.size-1MBiso-64KBsize-1MBowned unmappeddisappears from the region table entirely; host footprint holds flat at 33.8 MB across the run. Linux throughput improves 91 → 100 ev/s.Ordering is preserved. Once a window sends one event by reference it routes all subsequent events through the same JS promise chain, so a small event cannot overtake a large one still being fetched. Verified with an
interleavescenario alternating 1 KB and 64 KB — consecutive sequence numbers travelling by different mechanisms — at 0 reorders, 0 drops on both platforms.No public API change and no runtime JS change. All three delivery templates call the same
window._wails.dispatchWailsEventand keep the existing guard for the runtime not yet being mounted.Store hygiene: one-shot on read, bound to the dispatching window via the existing
x-wails-window-idheader, TTL-reaped (sweeping at TTL/4 so the 30 s bound actually holds), 64 MB cap with fallback to inline on overflow, released on window destroy, and the reaper stopped viaOnShutdown.Not fixed here
dispatchOnMainThreadruns inline when already on the main thread, so a main-thread emit can overtake a queued goroutine emit. Pre-existing, unrelated to payload size, unaffected by this change. Themixedsourcescenario is included as its regression test.Type of change
How Has This Been Tested?
New harness at
v3/tests/event-performance— a standalone Wails app that drives events straight atDispatchWailsEvent, samples both the host and web content processes every 250 ms, and grades on floor rise (min of second half − min of first half) rather than a regression slope, since memory under GC is a sawtooth and a slope mostly reports which phase the window opened in.Roughly 1.9 M events on macOS and 1.2 M on Linux across the full matrix, plus the before/after and attribution runs.
Linux: Ubuntu 26.04 LTS, x86_64, webkit2gtk-4.1 2.52.3 (linked GTK4 / webkitgtk-6.0), headless under Xvfb, Go 1.26.2.
Windows: Windows 11 Pro 26200, x86_64, WebView2 runtime 151.0.4129.72, Go 1.26.2, MinGW-w64 gcc 15.2, run in the interactive console session.
WebView2 does not have this bug
The host process holds flat at 65–77 MB unpatched, where macOS reached 11,629 MB. The content process sawtooths (climb to ~4 GB, collapse, repeat) in both patched and unpatched builds, so that is V8 reclaiming the 1 MB objects the events genuinely carry, not transport retention. WebKitGTK by contrast climbed monotonically to 6.2 GB and never recovered.
The change still helps on Windows, for a different reason — isolated 90 s runs, fresh process each:
More bytes delivered at roughly half the accumulation rate, which fits: an eval materialises both the 1 MB source string and the parsed object, a fetch only the object. Throughput is the firmer number; the ~2× is one run against a 4 GB sawtooth and should be read as directional.
Honest cost on Windows: the host runs higher patched (146–233 MB vs 65–77 MB) from payloads parked awaiting fetch plus Go GC lag. Bounded, but a real trade.
Note the 28-scenario matrix is not usable on Chromium — V8 never returns to a stable floor, so growth carries across scenarios and per-scenario attribution breaks down. Only isolated fresh-process runs are reported for Windows. The floor-rise metric is calibrated to WebKit and needs rethinking for Chromium.
Test Configuration
Checklist:
website/src/pages/changelog.mdxwith details of this PR (v3 changelog entries are added automatically)Summary by CodeRabbit
New Features
Bug Fixes
Tests