Skip to content

Commit 7ee25c5

Browse files
leaanthonyleaanthony
committed
fix(v3): address Copilot review on #5192
- mainthread_android.go: apply the same RLock -> Lock change made to the linux/macOS/iOS callback paths. Android uses the same shared mainThreadFunctionStore, so it had the identical map-write under RLock pattern. - transport_event_ipc_test: the previous lock-release test used an empty windows map and so would also have passed against the broken (lock-held-across-dispatch) implementation. Replace it with a probe Window whose DispatchWailsEvent attempts windowsLock.TryLock(); the fixed snapshot-and-release path lets the TryLock succeed, a regression to holding RLock across per-window dispatch fails it. Verified by mutation-test against the regressed implementation. - test/4424-event-block: the manual repro had no webview window and so EventIPCTransport.DispatchWailsEvent's per-window loop was a no-op, making the harness unable to reach the ExecJS path involved in #4424. Add a small webview window that subscribes to the emitted event, bump the emit cadence to 100ms so menu clicks have a high probability of landing during a dispatch, and document the manual pass/fail criteria in the file header.
1 parent d28a99a commit 7ee25c5

4 files changed

Lines changed: 114 additions & 22 deletions

File tree

‎v3/pkg/application/mainthread_android.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,13 @@ func (a *androidApp) isOnMainThread() bool {
1313
func (a *androidApp) dispatchOnMainThread(id uint) {
1414
// TODO: Implement via JNI callback to Activity.runOnUiThread()
1515
// For now, execute the callback directly
16-
mainThreadFunctionStoreLock.RLock()
16+
mainThreadFunctionStoreLock.Lock()
1717
fn := mainThreadFunctionStore[id]
1818
if fn == nil {
19-
mainThreadFunctionStoreLock.RUnlock()
19+
mainThreadFunctionStoreLock.Unlock()
2020
return
2121
}
2222
delete(mainThreadFunctionStore, id)
23-
mainThreadFunctionStoreLock.RUnlock()
23+
mainThreadFunctionStoreLock.Unlock()
2424
fn()
2525
}

‎v3/pkg/application/transport_event_ipc_test.go‎

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,24 +6,55 @@ import (
66
"time"
77
)
88

9-
func TestDispatchWailsEventReleasesLockAfterCompletion(t *testing.T) {
9+
// lockProbeWindow is a minimal Window implementation that captures whether
10+
// app.windowsLock could be exclusively acquired during the per-window
11+
// dispatch. Embedding the Window interface gives nil-bodied placeholders
12+
// for every other method; DispatchWailsEvent is the only one called by
13+
// EventIPCTransport.DispatchWailsEvent, so the placeholders never run.
14+
type lockProbeWindow struct {
15+
Window
16+
app *App
17+
acquiredWriteLock bool
18+
}
19+
20+
func (w *lockProbeWindow) DispatchWailsEvent(_ *CustomEvent) {
21+
if w.app.windowsLock.TryLock() {
22+
w.acquiredWriteLock = true
23+
w.app.windowsLock.Unlock()
24+
}
25+
}
26+
27+
// TestDispatchWailsEventReleasesLockBeforePerWindowDispatch is the regression
28+
// test for the deadlock pattern from #5016 / #4424: holding windowsLock
29+
// across the per-window DispatchWailsEvent call blocks any goroutine that
30+
// needs the lock for write — including the main thread when ExecJS routes
31+
// through it. The fix snapshots the windows under RLock then releases
32+
// before iterating. A regression that re-acquires (or never releases)
33+
// the lock for the iteration would prevent the probe window from taking
34+
// the write lock and fail this test.
35+
func TestDispatchWailsEventReleasesLockBeforePerWindowDispatch(t *testing.T) {
1036
app := &App{
1137
windows: make(map[uint]Window),
1238
windowsLock: sync.RWMutex{},
1339
}
40+
probe := &lockProbeWindow{app: app}
41+
app.windows[1] = probe
1442

1543
transport := &EventIPCTransport{app: app}
16-
event := &CustomEvent{Name: "test"}
44+
transport.DispatchWailsEvent(&CustomEvent{Name: "test"})
1745

18-
transport.DispatchWailsEvent(event)
46+
if !probe.acquiredWriteLock {
47+
t.Fatal("windowsLock was held during per-window DispatchWailsEvent; " +
48+
"the snapshot-and-release pattern in EventIPCTransport.DispatchWailsEvent has regressed")
49+
}
1950

51+
// Sanity-check: the lock is also released after the whole transport call.
2052
acquired := make(chan struct{})
2153
go func() {
2254
app.windowsLock.Lock()
2355
close(acquired)
2456
app.windowsLock.Unlock()
2557
}()
26-
2758
select {
2859
case <-acquired:
2960
case <-time.After(time.Second):

‎v3/test/4424-event-block/assets/index.html‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<!doctype html>
2+
<html lang="en">
3+
<head>
4+
<meta charset="utf-8">
5+
<title>Event Block Test (#4424)</title>
6+
<style>
7+
body { font: 14px system-ui, sans-serif; padding: 16px; }
8+
h1 { font-size: 16px; margin: 0 0 8px; }
9+
pre { background: #f4f4f4; padding: 8px; border-radius: 4px; }
10+
.counter { font-variant-numeric: tabular-nums; font-size: 28px; }
11+
</style>
12+
</head>
13+
<body>
14+
<h1>Manual reproduction for issue #4424</h1>
15+
<p>Backend events arrive here; menu clicks should still respond.</p>
16+
<div class="counter" id="counter">waiting&hellip;</div>
17+
<pre id="last"></pre>
18+
<script type="module">
19+
import { Events } from "/wails/runtime.js";
20+
let last = 0;
21+
Events.On("backendmessage", (ev) => {
22+
const data = ev.data;
23+
document.getElementById("counter").textContent = "#" + data.counter;
24+
document.getElementById("last").textContent = data.time;
25+
last = data.counter;
26+
});
27+
</script>
28+
</body>
29+
</html>

‎v3/test/4424-event-block/main.go‎

Lines changed: 47 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,26 +10,61 @@ import (
1010
"github.com/wailsapp/wails/v3/pkg/icons"
1111
)
1212

13+
// Manual reproduction harness for #4424.
14+
//
15+
// The original bug surfaces as menu/systray clicks freezing while the
16+
// backend is busy emitting events. The freeze happens because the event
17+
// dispatch path holds windowsLock during a per-window ExecJS call, which
18+
// blocks any goroutine that needs windowsLock for write — including the
19+
// main thread when it processes the menu activation.
20+
//
21+
// To reproduce, EventIPCTransport.DispatchWailsEvent must actually reach
22+
// the per-window ExecJS path; that requires (a) at least one webview
23+
// window registered on the App and (b) the event-emit goroutine running
24+
// frequently enough that a menu click is likely to land inside one of
25+
// the per-window dispatches.
26+
//
27+
// Manual procedure:
28+
// 1. Run the binary.
29+
// 2. A visible window opens showing the incrementing counter (proves
30+
// events round-trip through the per-window dispatch path).
31+
// 3. Right-click the systray icon and click each menu item repeatedly.
32+
// 4. With the fix in place, menu items respond instantly even while
33+
// the counter is ticking. Without the fix, clicks queue up or are
34+
// ignored until the event burst stops.
35+
//
36+
//go:embed assets/index.html
37+
var indexHTML []byte
38+
1339
func main() {
1440
app := application.New(application.Options{
1541
Name: "Event Block Test",
16-
Description: "Test for issue #4424 - Event emitter blocks menu button press",
42+
Description: "Manual reproduction for #4424 — event emitter blocks menu clicks on Linux",
1743
Assets: application.AlphaAssets,
1844
Mac: application.MacOptions{
1945
ActivationPolicy: application.ActivationPolicyAccessory,
2046
},
2147
})
2248

23-
// Create a system tray with a menu
49+
// A visible window is required: EventIPCTransport.DispatchWailsEvent
50+
// iterates over app.windows and calls ExecJS on each. With no window
51+
// registered, the dispatch loop is a no-op and the bug cannot reproduce.
52+
app.Window.NewWithOptions(application.WebviewWindowOptions{
53+
Title: "Event Block Test (#4424)",
54+
Width: 480,
55+
Height: 240,
56+
HTML: string(indexHTML),
57+
})
58+
2459
systemTray := app.SystemTray.New()
2560
systemTray.SetIcon(icons.WailsLogoBlack)
2661

2762
menu := app.NewMenu()
2863
menu.Add("About").OnClick(func(ctx *application.Context) {
2964
log.Println("About clicked!")
30-
application.InfoDialog().
65+
app.Dialog.Info().
3166
SetTitle("About").
32-
SetMessage("This is a test for issue #4424").
67+
SetMessage("Manual repro harness for issue #4424").
3368
Show()
3469
})
3570
menu.Add("Show Time").OnClick(func(ctx *application.Context) {
@@ -40,31 +75,28 @@ func main() {
4075
log.Println("Quit clicked!")
4176
app.Quit()
4277
})
43-
4478
systemTray.SetMenu(menu)
4579

46-
// Start emitting events after application starts
4780
app.Event.OnApplicationEvent(events.Common.ApplicationStarted, func(event *application.ApplicationEvent) {
4881
go func() {
49-
log.Println("Starting event emitter...")
50-
log.Println("TEST: Right-click the systray icon and click menu items.")
51-
log.Println("They should continue to work even with events being emitted.")
52-
log.Println("BUG: On Linux, menu clicks stop working after events start.")
82+
log.Println("Starting event emitter at 100ms intervals.")
83+
log.Println("TEST: right-click the systray icon and click menu items repeatedly.")
84+
log.Println("PASS: menu items respond immediately while the counter ticks.")
85+
log.Println("FAIL: menu clicks stall, queue up, or are dropped until the burst stops.")
5386

5487
counter := 0
55-
for {
88+
ticker := time.NewTicker(100 * time.Millisecond)
89+
defer ticker.Stop()
90+
for range ticker.C {
5691
counter++
57-
log.Printf("Emitting event #%d...\n", counter)
5892
app.Event.Emit("backendmessage", map[string]interface{}{
5993
"counter": counter,
60-
"time": time.Now().Format(time.RFC3339),
94+
"time": time.Now().Format(time.RFC3339Nano),
6195
})
62-
time.Sleep(3 * time.Second)
6396
}
6497
}()
6598
})
6699

67-
// Run the application
68100
if err := app.Run(); err != nil {
69101
log.Fatal(err)
70102
}

0 commit comments

Comments
 (0)