mirror of
https://github.com/netbirdio/netbird.git
synced 2026-09-12 17:59:06 +02:00
[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.
This commit is contained in:
@@ -131,6 +131,7 @@ type WindowManager struct {
|
||||
newMain func(startURL string) *application.WebviewWindow
|
||||
creating map[string]bool
|
||||
pendingOps map[string][]windowOp
|
||||
restoreGen uint64
|
||||
ready map[uint]bool
|
||||
showPending map[uint]bool
|
||||
pendingTab map[uint]string
|
||||
@@ -567,6 +568,11 @@ func (s *WindowManager) ensureMain(startURL string, op windowOp) {
|
||||
|
||||
func (s *WindowManager) withWindow(name string, slot **application.WebviewWindow, factory func() *application.WebviewWindow, op windowOp) {
|
||||
s.mu.Lock()
|
||||
if s.creating[name] {
|
||||
s.pendingOps[name] = append(s.pendingOps[name], op)
|
||||
s.mu.Unlock()
|
||||
return
|
||||
}
|
||||
if w := *slot; w != nil {
|
||||
s.mu.Unlock()
|
||||
op(w, false)
|
||||
@@ -576,29 +582,66 @@ func (s *WindowManager) withWindow(name string, slot **application.WebviewWindow
|
||||
s.mu.Unlock()
|
||||
return
|
||||
}
|
||||
if s.creating[name] {
|
||||
s.pendingOps[name] = append(s.pendingOps[name], op)
|
||||
s.mu.Unlock()
|
||||
return
|
||||
}
|
||||
s.creating[name] = true
|
||||
s.mu.Unlock()
|
||||
|
||||
w := factory()
|
||||
|
||||
s.mu.Lock()
|
||||
*slot = w
|
||||
delete(s.creating, name)
|
||||
queued := s.pendingOps[name]
|
||||
delete(s.pendingOps, name)
|
||||
s.mu.Unlock()
|
||||
|
||||
w := s.createWindow(name, slot, factory)
|
||||
if w == nil {
|
||||
return
|
||||
}
|
||||
s.finishCreation(name, w, op)
|
||||
}
|
||||
|
||||
func (s *WindowManager) createWindow(name string, slot **application.WebviewWindow, factory func() *application.WebviewWindow) *application.WebviewWindow {
|
||||
created := false
|
||||
defer func() {
|
||||
if created {
|
||||
return
|
||||
}
|
||||
s.mu.Lock()
|
||||
delete(s.creating, name)
|
||||
delete(s.pendingOps, name)
|
||||
s.mu.Unlock()
|
||||
}()
|
||||
|
||||
w := factory()
|
||||
if w == nil {
|
||||
return nil
|
||||
}
|
||||
s.mu.Lock()
|
||||
*slot = w
|
||||
s.mu.Unlock()
|
||||
created = true
|
||||
return w
|
||||
}
|
||||
|
||||
func (s *WindowManager) finishCreation(name string, w *application.WebviewWindow, op windowOp) {
|
||||
finished := false
|
||||
defer func() {
|
||||
if finished {
|
||||
return
|
||||
}
|
||||
s.mu.Lock()
|
||||
delete(s.creating, name)
|
||||
delete(s.pendingOps, name)
|
||||
s.mu.Unlock()
|
||||
}()
|
||||
|
||||
op(w, true)
|
||||
for _, queuedOp := range queued {
|
||||
queuedOp(w, false)
|
||||
for {
|
||||
s.mu.Lock()
|
||||
queued := s.pendingOps[name]
|
||||
delete(s.pendingOps, name)
|
||||
if len(queued) == 0 {
|
||||
delete(s.creating, name)
|
||||
finished = true
|
||||
s.mu.Unlock()
|
||||
return
|
||||
}
|
||||
s.mu.Unlock()
|
||||
for _, queuedOp := range queued {
|
||||
queuedOp(w, false)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -927,6 +970,10 @@ func (s *WindowManager) retitleAll() {
|
||||
}
|
||||
|
||||
func (s *WindowManager) hideOtherWindows(keepName string) {
|
||||
s.mu.Lock()
|
||||
gen := s.restoreGen
|
||||
s.mu.Unlock()
|
||||
|
||||
var hidden []application.Window
|
||||
for _, w := range s.app.Window.GetAll() {
|
||||
if w == nil || w.Name() == keepName || !w.IsVisible() {
|
||||
@@ -938,9 +985,19 @@ func (s *WindowManager) hideOtherWindows(keepName string) {
|
||||
if len(hidden) == 0 {
|
||||
return
|
||||
}
|
||||
|
||||
s.mu.Lock()
|
||||
s.hiddenForLogin = append(s.hiddenForLogin, hidden...)
|
||||
restored := s.restoreGen != gen
|
||||
if !restored {
|
||||
s.hiddenForLogin = append(s.hiddenForLogin, hidden...)
|
||||
}
|
||||
s.mu.Unlock()
|
||||
if !restored {
|
||||
return
|
||||
}
|
||||
for _, w := range hidden {
|
||||
w.Show()
|
||||
}
|
||||
}
|
||||
|
||||
// restoreHiddenWindows re-shows windows hidden by hideOtherWindows. If the main
|
||||
@@ -951,6 +1008,7 @@ func (s *WindowManager) restoreHiddenWindows() {
|
||||
s.mu.Lock()
|
||||
hidden := s.hiddenForLogin
|
||||
s.hiddenForLogin = nil
|
||||
s.restoreGen++
|
||||
mainWindow := s.mainWindow
|
||||
s.mu.Unlock()
|
||||
|
||||
|
||||
@@ -125,3 +125,73 @@ func TestWithWindowConcurrentCallersShareOneCreation(t *testing.T) {
|
||||
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)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user