diff --git a/client/ui/services/windowmanager.go b/client/ui/services/windowmanager.go index 68542ae53..d93b25821 100644 --- a/client/ui/services/windowmanager.go +++ b/client/ui/services/windowmanager.go @@ -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() diff --git a/client/ui/services/windowmanager_test.go b/client/ui/services/windowmanager_test.go index c98d2db2a..9de9f13b8 100644 --- a/client/ui/services/windowmanager_test.go +++ b/client/ui/services/windowmanager_test.go @@ -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) +}