Files
netbird/client/ui/services/windowmanager_test.go
T
Zoltan Papp 7da0c2a07a [client] Fix the Windows tray deadlock on re-entrant window creation (#7449) (#7537)
* [client] Fix the Windows tray deadlock on re-entrant window creation

The Wails systray runs the left-click handler synchronously inside the
tray window procedure, and creating a window on a running app pumps a
nested Win32 message loop while WebView2 initialises. ensureWindow held
the non-reentrant createMu across that creation, so the second button-up
of a double click re-entered ShowWindow from the pump and blocked the
main thread on its own lock. A goroutine holding createMu while the main
thread pumped, and the Open* dialogs holding mu across NewWithOptions,
Show, Hide and InvokeSync, exposed the same inversion.

WindowManager now serialises creation with a per-slot creating flag and
queues the callers' operations until the window exists, and no Wails call
runs while mu is held. The tray click and second-instance handlers call
ShowWindow off the message loop.

* [client] Serialize window operations while a slot is being created

Callers arriving after the window is published but before the creator
has drained the queue took the existing-window fast path and could run
ahead of older queued operations, so a newer SetURL could be overwritten
by an older one. withWindow now queues every caller while the creating
flag is set and clears the flag only once the queue is seen empty under
the lock.

A factory panic or a nil window left the creating flag set and the slot
dead; creation and drain now reset that state on early exit.

hideOtherWindows records the windows it hid only when no restore ran
in between, tracked by a generation counter, and re-shows them otherwise,
so a restore racing the hide cannot strand hidden windows.

* [misc] Run the client/ui subpackage tests in CI

The three test workflows filtered the package list with a `/client/ui`
prefix match, which dropped the subpackages along with the package that
cannot compile without a frontend build. `services`, `preferences`,
`i18n` and `authsession` all carry Go-side unit tests that never ran,
including the window manager re-entrancy regression test.

Anchor the pattern so only `client/ui` itself is excluded. The linux leg
keeps the prefix match on 386, where only the 64-bit gtk4/webkitgtk dev
packages are installed and the Wails application package would fail to
link, and the alpine container job keeps it for the same reason.

* [misc] Run the client/ui subpackage tests on a gtk4 4.10 runner

The previous commit let the subpackages into the linux client job, where
client/ui/services failed to build: the wails runtime's linux cgo layer
uses GtkFileDialog, which arrived in gtk4 4.10, and the job's ubuntu-22.04
runner ships 4.6.

Move them to their own job pinned to ubuntu-24.04 and restore the linux
client job's original exclusion, leaving the 386 and privileged legs on
the runner they have used since 2024. The new job needs no build cache,
sudo or privileged tag, so it stays a few seconds long.

Darwin and Windows keep the anchored pattern from the previous commit and
already run these tests green, including the window manager re-entrancy
regression test on the platform the deadlock was reported on.

* [client] Defer a window close that lands while the window is still being created

WindowManager publishes a dialog's slot only after the factory returns,
and on Windows the factory blocks in the WebView2 embed pump. A Close*
arriving in that gap found a nil slot and returned without doing
anything, so the dialog appeared afterwards for a flow that had already
been cancelled. The pre-fix Open* dialog functions held mu across the
whole creation, which blocked a concurrent Close* until the slot was
set; removing that lock hold reopened this gap.

Close* now goes through closeWindow: while the slot is being created it
records a closer in pendingClose, and finishCreation runs that closer
before any queued operation, so a window that is going away is never
shown and Wails never sees a Show on a destroyed window, which would
recreate it. Ops queued behind a close are dropped; windowOp carries no
factory, so they cannot be replayed into a new creation, and the
frontend callers reissue on the next state change.

The browser-login slot uses the same restoring closer from both
CloseBrowserLogin and CloseRenewFlow, since the popup's WindowClosing
hook only restores on a user close. Where two closers race one
creation the first registered wins, so a later caller cannot replace a
restoring closer with one that does not restore.

(cherry picked from commit 2d28f9002a)
2026-09-14 18:06:35 +02:00

351 lines
11 KiB
Go

//go:build !android && !ios && !freebsd && !js
package services
import (
"fmt"
"sync/atomic"
"testing"
"time"
"github.com/stretchr/testify/require"
"github.com/wailsapp/wails/v3/pkg/application"
)
func newTestWindowManager() *WindowManager {
return &WindowManager{
creating: map[string]bool{},
pendingOps: map[string][]windowOp{},
pendingClose: map[string]windowCloser{},
}
}
func waitDone(t *testing.T, done <-chan struct{}, msg string) {
t.Helper()
select {
case <-done:
case <-time.After(5 * time.Second):
t.Fatal(msg)
}
}
func TestWithWindowReusesExistingWindow(t *testing.T) {
s := newTestWindowManager()
existing := &application.WebviewWindow{}
slot := existing
factoryCalls := 0
var got *application.WebviewWindow
created := true
s.withWindow(windowMain, &slot, func() *application.WebviewWindow {
factoryCalls++
return &application.WebviewWindow{}
}, func(w *application.WebviewWindow, c bool) {
got, created = w, c
})
require.Equal(t, 0, factoryCalls)
require.Same(t, existing, got)
require.False(t, created)
}
func TestWithWindowNilFactoryWithoutWindowSkipsOp(t *testing.T) {
s := newTestWindowManager()
var slot *application.WebviewWindow
opCalls := 0
s.withWindow(windowMain, &slot, nil, func(*application.WebviewWindow, bool) {
opCalls++
})
require.Equal(t, 0, opCalls)
require.Nil(t, slot)
}
func TestWithWindowReentrantCallDuringCreationIsQueued(t *testing.T) {
s := newTestWindowManager()
var slot *application.WebviewWindow
factoryCalls := 0
var order []string
var factory func() *application.WebviewWindow
factory = func() *application.WebviewWindow {
factoryCalls++
// Simulates the Windows message pump re-entering the tray click handler
// while WebView2 is still initialising the window being created.
s.withWindow(windowMain, &slot, factory, func(_ *application.WebviewWindow, created bool) {
order = append(order, fmt.Sprintf("reentrant:%v", created))
})
return &application.WebviewWindow{}
}
done := make(chan struct{})
go func() {
defer close(done)
s.withWindow(windowMain, &slot, factory, func(_ *application.WebviewWindow, created bool) {
order = append(order, fmt.Sprintf("outer:%v", created))
})
}()
waitDone(t, done, "withWindow deadlocked on a re-entrant call during creation")
require.Equal(t, 1, factoryCalls)
require.Equal(t, []string{"outer:true", "reentrant:false"}, order)
require.NotNil(t, slot)
require.Empty(t, s.creating)
require.Empty(t, s.pendingOps)
}
func TestWithWindowConcurrentCallersShareOneCreation(t *testing.T) {
s := newTestWindowManager()
var slot *application.WebviewWindow
factoryEntered := make(chan struct{})
release := make(chan struct{})
var factoryCalls, opCalls atomic.Int32
factory := func() *application.WebviewWindow {
factoryCalls.Add(1)
close(factoryEntered)
<-release
return &application.WebviewWindow{}
}
op := func(*application.WebviewWindow, bool) { opCalls.Add(1) }
first := make(chan struct{})
go func() {
defer close(first)
s.withWindow(windowSettings, &slot, factory, op)
}()
<-factoryEntered
second := make(chan struct{})
go func() {
defer close(second)
s.withWindow(windowSettings, &slot, factory, op)
}()
waitDone(t, second, "second caller blocked while the window was being created")
require.Equal(t, int32(0), opCalls.Load())
close(release)
waitDone(t, first, "creator did not finish")
require.Equal(t, int32(1), factoryCalls.Load())
require.Equal(t, int32(2), opCalls.Load())
require.NotNil(t, slot)
}
func TestWithWindowOpsQueuedDuringCreationRunInArrivalOrder(t *testing.T) {
s := newTestWindowManager()
var slot *application.WebviewWindow
var order []string
record := func(label string) windowOp {
return func(_ *application.WebviewWindow, created bool) {
order = append(order, fmt.Sprintf("%s:%v", label, created))
}
}
var factory func() *application.WebviewWindow
factory = func() *application.WebviewWindow {
s.withWindow(windowMain, &slot, factory, func(w *application.WebviewWindow, created bool) {
record("a")(w, created)
// Arrives while the creator is still draining the queue: it must not
// jump ahead of "b" through the existing-window fast path.
s.withWindow(windowMain, &slot, factory, record("c"))
})
s.withWindow(windowMain, &slot, factory, record("b"))
return &application.WebviewWindow{}
}
done := make(chan struct{})
go func() {
defer close(done)
s.withWindow(windowMain, &slot, factory, record("outer"))
}()
waitDone(t, done, "withWindow deadlocked while draining queued operations")
require.Equal(t, []string{"outer:true", "a:false", "b:false", "c:false"}, order)
require.Empty(t, s.creating)
require.Empty(t, s.pendingOps)
}
func TestWithWindowFactoryPanicReleasesCreation(t *testing.T) {
s := newTestWindowManager()
var slot *application.WebviewWindow
func() {
defer func() { require.NotNil(t, recover()) }()
s.withWindow(windowMain, &slot, func() *application.WebviewWindow {
panic("factory failed")
}, func(*application.WebviewWindow, bool) {})
}()
require.Empty(t, s.creating)
require.Empty(t, s.pendingOps)
require.Nil(t, slot)
created := false
s.withWindow(windowMain, &slot, func() *application.WebviewWindow {
return &application.WebviewWindow{}
}, func(_ *application.WebviewWindow, c bool) {
created = c
})
require.True(t, created)
require.NotNil(t, slot)
}
func TestWithWindowNilFromFactoryReleasesCreation(t *testing.T) {
s := newTestWindowManager()
var slot *application.WebviewWindow
opCalls := 0
s.withWindow(windowMain, &slot, func() *application.WebviewWindow {
return nil
}, func(*application.WebviewWindow, bool) {
opCalls++
})
require.Equal(t, 0, opCalls)
require.Empty(t, s.creating)
require.Nil(t, slot)
}
func TestCloseWindowDuringCreationDefersCloseAndSkipsOps(t *testing.T) {
s := newTestWindowManager()
var slot *application.WebviewWindow
created := &application.WebviewWindow{}
opCalls, closeCalls := 0, 0
var closed *application.WebviewWindow
s.withWindow(windowError, &slot, func() *application.WebviewWindow {
s.closeWindow(windowError, &slot, func(w *application.WebviewWindow) {
closeCalls++
closed = w
})
require.Equal(t, 0, closeCalls)
return created
}, func(*application.WebviewWindow, bool) {
opCalls++
})
require.Equal(t, 0, opCalls)
require.Equal(t, 1, closeCalls)
require.Same(t, created, closed)
require.Nil(t, slot)
require.Empty(t, s.creating)
require.Empty(t, s.pendingOps)
require.Empty(t, s.pendingClose)
factoryCalls := 0
reopened := false
s.withWindow(windowError, &slot, func() *application.WebviewWindow {
factoryCalls++
return &application.WebviewWindow{}
}, func(_ *application.WebviewWindow, c bool) {
reopened = c
})
require.Equal(t, 1, factoryCalls)
require.True(t, reopened)
require.NotNil(t, slot)
}
func TestCloseWindowDuringDrainStopsRemainingOps(t *testing.T) {
s := newTestWindowManager()
var slot *application.WebviewWindow
var order []string
closeCalls := 0
var factory func() *application.WebviewWindow
factory = func() *application.WebviewWindow {
s.withWindow(windowWelcome, &slot, factory, func(*application.WebviewWindow, bool) {
order = append(order, "a")
s.closeWindow(windowWelcome, &slot, func(*application.WebviewWindow) { closeCalls++ })
s.withWindow(windowWelcome, &slot, factory, func(*application.WebviewWindow, bool) {
order = append(order, "c")
})
})
s.withWindow(windowWelcome, &slot, factory, func(*application.WebviewWindow, bool) {
order = append(order, "b")
})
return &application.WebviewWindow{}
}
s.withWindow(windowWelcome, &slot, factory, func(*application.WebviewWindow, bool) {
order = append(order, "outer")
})
// "b" was queued before the close and "c" after it; a close supersedes both
// rather than showing a window that is about to be destroyed.
require.Equal(t, []string{"outer", "a"}, order)
require.Equal(t, 1, closeCalls)
require.Nil(t, slot)
require.Empty(t, s.creating)
require.Empty(t, s.pendingOps)
require.Empty(t, s.pendingClose)
}
func TestCloseWindowWithoutWindowSkipsCloser(t *testing.T) {
s := newTestWindowManager()
var slot *application.WebviewWindow
calls := 0
s.closeWindow(windowBrowserLogin, &slot, func(*application.WebviewWindow) { calls++ })
require.Equal(t, 0, calls)
require.Nil(t, slot)
require.Empty(t, s.pendingClose)
}
func TestCloseWindowWithExistingWindowRunsCloser(t *testing.T) {
s := newTestWindowManager()
existing := &application.WebviewWindow{}
slot := existing
var got *application.WebviewWindow
s.closeWindow(windowError, &slot, func(w *application.WebviewWindow) { got = w })
require.Same(t, existing, got)
require.Nil(t, slot)
require.Empty(t, s.pendingClose)
}
func TestWithWindowNilFromFactoryDropsPendingClose(t *testing.T) {
s := newTestWindowManager()
var slot *application.WebviewWindow
closeCalls := 0
s.withWindow(windowError, &slot, func() *application.WebviewWindow {
s.closeWindow(windowError, &slot, func(*application.WebviewWindow) { closeCalls++ })
return nil
}, func(*application.WebviewWindow, bool) {})
require.Equal(t, 0, closeCalls)
require.Nil(t, slot)
require.Empty(t, s.creating)
require.Empty(t, s.pendingClose)
}
func TestWithWindowFactoryPanicDropsPendingClose(t *testing.T) {
s := newTestWindowManager()
var slot *application.WebviewWindow
closeCalls := 0
func() {
defer func() { require.NotNil(t, recover()) }()
s.withWindow(windowError, &slot, func() *application.WebviewWindow {
s.closeWindow(windowError, &slot, func(*application.WebviewWindow) { closeCalls++ })
panic("factory failed")
}, func(*application.WebviewWindow, bool) {})
}()
require.Equal(t, 0, closeCalls)
require.Empty(t, s.creating)
require.Empty(t, s.pendingClose)
}
func TestCloseWindowKeepsFirstDeferredCloser(t *testing.T) {
s := newTestWindowManager()
var slot *application.WebviewWindow
var ran []string
s.withWindow(windowError, &slot, func() *application.WebviewWindow {
s.closeWindow(windowError, &slot, func(*application.WebviewWindow) { ran = append(ran, "first") })
s.closeWindow(windowError, &slot, func(*application.WebviewWindow) { ran = append(ran, "second") })
return &application.WebviewWindow{}
}, func(*application.WebviewWindow, bool) {})
require.Equal(t, []string{"first"}, ran)
require.Nil(t, slot)
require.Empty(t, s.pendingClose)
}
func TestCloseRenewFlowDuringBrowserLoginCreationRestoresHiddenWindows(t *testing.T) {
s := newTestWindowManager()
s.withWindow(windowBrowserLogin, &s.browserLogin, func() *application.WebviewWindow {
s.CloseRenewFlow()
// Seeded after the call so the deferred closer, not CloseRenewFlow's own
// immediate restore, is what has to drain it. A nil entry is skipped by
// restoreHiddenWindows, so no Wails window is needed.
s.hiddenForLogin = []application.Window{nil}
return &application.WebviewWindow{}
}, func(*application.WebviewWindow, bool) {})
require.Nil(t, s.browserLogin)
require.Empty(t, s.hiddenForLogin)
require.Empty(t, s.creating)
require.Empty(t, s.pendingClose)
}