[v3] (*Chromium).Focus race on production startup — #4835 fix has TOCTOU window

[haohow123]

Description

Hitting the same crash signature as #4835 in alpha.90 production build (not dev mode). The fix from #4835 (commit 464659b — `GetController() != nil` guards in `webview_window_windows.go`) is present in our build, but the check and the subsequent `Focus()` call are non-atomic: in our crash trace the controller has been torn down between the guard at line 1023 and the call at line 1028.

Stack trace (production launcher.log, ~4 s after `App.Run`)

```
[WebView2 Error] The parameter is incorrect.

1: github.com/wailsapp/wails/webview2/pkg/edge.(*Chromium).errorCallback
github.com/wailsapp/wails/webview2@v1.0.24/pkg/edge/chromium.go:151
2: github.com/wailsapp/wails/webview2/pkg/edge.(*Chromium).Focus
github.com/wailsapp/wails/webview2@v1.0.24/pkg/edge/chromium.go:575
3: github.com/wailsapp/wails/v3/pkg/application.(*windowsWebviewWindow).focus
github.com/wailsapp/wails/v3@v3.0.0-alpha.90/pkg/application/webview_window_windows.go:1028
4: github.com/wailsapp/wails/v3/pkg/application.(*windowsWebviewWindow).WndProc
github.com/wailsapp/wails/v3@v3.0.0-alpha.90/pkg/application/webview_window_windows.go:1546
5: github.com/wailsapp/wails/v3/pkg/application.(*windowsApp).wndProc
github.com/wailsapp/wails/v3@v3.0.0-alpha.90/pkg/application/application_windows.go:303
```

(Identical shape to the trace in #4835, just from a production binary instead of `wails3 dev` hot-reload.)

Race

`v3/pkg/application/webview_window_windows.go` alpha.90:

```go
1009: func (w *windowsWebviewWindow) focus() {
...
1023: if w.chromium.GetController() == nil {
1024: return
1025: }
// ── another goroutine can tear the controller down here ──
1028: w.chromium.Focus() // crashes when controller is now nil/destroyed
}
```

The other three `chromium.Focus()` call sites in the same file (lines 884–885, 896–897, 945–946) have the same shape — `GetController() != nil` then call — so the same TOCTOU window exists for each.

Trigger

Not deterministic — surfaces ~1 in 10 cold launches of a Wails v3 launcher on Windows 11, ~4 s after `App.Run`. The window message landing in `WndProc` while WebView2's controller is still initialising seems to be the trigger. dev hot-reload (the #4835 reproducer) is not involved.

Suggested fix direction

1. Snapshot the controller pointer under `w.chromium`'s lock inside `focus()` (and the other three sites) and call `Focus()` only on the snapshot, OR
2. Defer focus messages until the controller has finished initialising (e.g. queue them and drain after `WebView2 Environment created successfully` fires).

Either keeps the fix local to the affected call sites without changing the broader controller lifecycle.

Versions

• Wails v3: `v3.0.0-alpha.90`
• Windows 10/11
• WebView2 runtime: whatever Windows ships
• Build: production (`-tags production -H windowsgui`), tag-triggered release workflow

Cross-ref: #4835 (closed, partial fix landed in 464659b — covers nil controller but not the race).

[taliesin-ai]

Code Review: TOCTOU Race in WebView2 Focus Handling

Thanks for the detailed write-up. The race is real and the report is accurate. Here's the analysis with exact evidence from the current master source, plus a concrete fix recommendation.

---

The exact race window

CreateCoreWebView2ControllerCompleted in webview2/pkg/edge/chromium.go:

// line 338  ← e.controller is now NON-NIL
e.controller = controller

// lines 339–388: controller3 bounds mode, GetCoreWebView2, AddWebMessageReceived,
//                AddPermissionRequested, AddWebResourceRequested, AddNavigationCompleted,
//                AddProcessFailed, AddContainsFullScreenElementChanged,
//                AddAcceleratorKeyPressed …

// line 389  ← inited flips to 1 ONLY after all setup is complete
atomic.StoreUintptr(&e.inited, 1)

The message pump in Embed() (lines 195–210) is running during this callback. Any COM call in lines 339–388 that internally pumps the Windows message queue can re-entrantly dispatch a WM_SETFOCUS to the WndProc before line 389 completes.

WM_SETFOCUS is handled directly in the WndProc (line 1545–1546 of webview_window_windows.go):

case w32.WM_SETFOCUS:
    w.focus()  // called directly, not via InvokeSync

At that moment:

• GetController() returns the non-nil pointer set at line 338 ✓
• But inited is still 0 — setup is incomplete ✗
• Focus() calls e.controller.MoveFocus() on a partially-configured controller
• WebView2 returns E_INVALIDARG ("The parameter is incorrect")
• errorCallback calls os.Exit(1) → crash

The ~4-second timing matches the WebView2 first-run installation delay, which is exactly when the startup message pump runs the longest.

There is direct precedent that this class of re-entrant WndProc dispatch is known in the codebase — NotifyParentWindowPositionChanged already carries the comment:

"It looks like the wndproc function is called before the controller initialization is complete. Because of this the controller is nil"

The difference is that WM_MOVE fires before e.controller is assigned, so the nil check there is sufficient. WM_SETFOCUS fires in the gap between e.controller = controller (line 338) and atomic.StoreUintptr(&e.inited, 1) (line 389), so the nil check is not sufficient.

---

Evaluating the two proposed fixes

Option 1 — Snapshot the controller pointer under a lock

Adding a sync.RWMutex to guard e.controller protects the Go pointer but does not protect the underlying COM object lifetime. A COM call to a controller whose internal state is still being configured will still return E_INVALIDARG. This approach closes the teardown race (where e.controller is non-nil but the COM object was released), but not the startup race described above.

Option 2 — Defer focus until initialization is fully complete

This is the right approach. inited is set atomically after all controller setup succeeds, making it the correct sentinel. Replacing GetController() != nil with a check against inited closes the startup race completely.

---

Recommended fix

Step 1 — Add IsReady() to the Chromium struct (webview2/pkg/edge/chromium.go):

// IsReady reports whether the WebView2 controller has been fully initialised.
// It is safe to call from any goroutine.
func (e *Chromium) IsReady() bool {
    return atomic.LoadUintptr(&e.inited) != 0
}

Step 2 — Guard Focus() itself (webview2/pkg/edge/chromium.go):

func (e *Chromium) Focus() {
    if !e.IsReady() {
        return
    }
    err := e.controller.MoveFocus(COREWEBVIEW2_MOVE_FOCUS_REASON_PROGRAMMATIC)
    if err != nil {
        e.errorCallback(err)
    }
}

This is defence-in-depth: Focus() becomes unconditionally safe regardless of call site.

Step 3 — Replace the four TOCTOU patterns in webview_window_windows.go:

// maximise() line 884
if w.chromium.IsReady() {
    w.chromium.Focus()
}

// restore() line 896
if w.chromium.IsReady() {
    w.chromium.Focus()
}

// fullscreen() line 945
if w.chromium.IsReady() {
    w.chromium.Focus()
}

// focus() lines 1023-1025
if !w.chromium.IsReady() {
    return
}

Step 4 — Cover the teardown case (webview2/pkg/edge/chromium.go):

When the controller is being destroyed, flip inited to 0 before releasing the COM object so any concurrent IsReady() check sees the controller as not ready. Add a Destroy() or amend the existing shutdown path:

func (e *Chromium) Destroy() {
    atomic.StoreUintptr(&e.inited, 0)   // gate off Focus() first
    if e.controller != nil {
        e.controller.vtbl.Release.Call(uintptr(unsafe.Pointer(e.controller)))
        e.controller = nil
    }
}

---

Why not a mutex?

A sync.RWMutex protects the Go-side pointer but not the COM object's internal state. The startup race exists between the pointer assignment (line 338) and the completion of COM setup (line 389); a mutex gating on the pointer would not cover that window. The inited atomic flag was introduced precisely to mark "fully ready", which is the correct predicate.

---

Testing recommendation

The crash is deterministic under slow WebView2 installation (first run) or simulated with:

// In a test harness: call window.Focus() immediately after App.Run()
// before WebView2 finishes initialising, with -race to catch data races
go func() { app.Run() }()
time.Sleep(100 * time.Millisecond) // before inited flips
w.Focus()

Verifying that Focus() silently no-ops instead of crashing is the acceptance criterion.

CC @leaanthony

---

Taliesin is an AI agent. CC @leaanthony