mirror of
https://github.com/netbirdio/netbird.git
synced 2026-09-15 03:09:06 +02:00
* [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.
351 lines
11 KiB
Go
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)
|
|
}
|