From 0b7d1f4f6718edd896a26247a1b793c30c384cee Mon Sep 17 00:00:00 2001 From: Atterpac <89053530+atterpac@users.noreply.github.com> Date: Tue, 17 Jun 2025 15:31:55 -0600 Subject: [PATCH] fix incorrect window destroy (#4323) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix incorrect window destroy * remove duplicate common signals * Better handling of "shouldclose" * Better handling of "shouldclose" * Fix ABI safety issues in exported function for macOS window closing Address CodeRabbit feedback on ABI safety and data races: - Change exported function signature to use C types (C.uint, C.bool) for ABI safety - Convert unconditionallyClose field from bool to atomic uint32 for thread safety - Update all read/write operations to use atomic operations across platforms 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --------- Co-authored-by: Lea Anthony Co-authored-by: Claude --- v3/pkg/application/webview_window.go | 7 ++--- .../webview_window_close_darwin.go | 26 +++++++++++++++++++ v3/pkg/application/webview_window_darwin.go | 9 +++++-- v3/pkg/application/webview_window_darwin.m | 14 ++++++++++ v3/pkg/application/webview_window_windows.go | 2 +- v3/pkg/events/defaults.go | 3 --- 6 files changed, 52 insertions(+), 9 deletions(-) create mode 100644 v3/pkg/application/webview_window_close_darwin.go diff --git a/v3/pkg/application/webview_window.go b/v3/pkg/application/webview_window.go index 374c423b7..a47b440b4 100644 --- a/v3/pkg/application/webview_window.go +++ b/v3/pkg/application/webview_window.go @@ -7,6 +7,7 @@ import ( "slices" "strings" "sync" + "sync/atomic" "text/template" "github.com/leaanthony/u" @@ -165,8 +166,8 @@ type WebviewWindow struct { // pendingJS holds JS that was sent to the window before the runtime was loaded pendingJS []string - // unconditionallyClose marks the window to be unconditionally closed - unconditionallyClose bool + // unconditionallyClose marks the window to be unconditionally closed (atomic) + unconditionallyClose uint32 } func (w *WebviewWindow) SetMenu(menu *Menu) { @@ -273,7 +274,7 @@ func NewWindow(options WebviewWindowOptions) *WebviewWindow { // Listen for window closing events and de result.OnWindowEvent(events.Common.WindowClosing, func(event *WindowEvent) { - result.unconditionallyClose = true + atomic.StoreUint32(&result.unconditionallyClose, 1) InvokeSync(result.markAsDestroyed) InvokeSync(result.impl.close) globalApplication.deleteWindowByID(result.id) diff --git a/v3/pkg/application/webview_window_close_darwin.go b/v3/pkg/application/webview_window_close_darwin.go new file mode 100644 index 000000000..6723d6f4a --- /dev/null +++ b/v3/pkg/application/webview_window_close_darwin.go @@ -0,0 +1,26 @@ +//go:build darwin + +package application + +/* +#include +*/ +import "C" +import "sync/atomic" + +//export windowShouldUnconditionallyClose +func windowShouldUnconditionallyClose(windowId C.uint) C.bool { + window := globalApplication.getWindowForID(uint(windowId)) + if window == nil { + globalApplication.debug("windowShouldUnconditionallyClose: window not found", "windowId", windowId) + return C.bool(false) + } + webviewWindow, ok := window.(*WebviewWindow) + if !ok { + globalApplication.debug("windowShouldUnconditionallyClose: window is not WebviewWindow", "windowId", windowId) + return C.bool(false) + } + unconditionallyClose := atomic.LoadUint32(&webviewWindow.unconditionallyClose) != 0 + globalApplication.debug("windowShouldUnconditionallyClose check", "windowId", windowId, "unconditionallyClose", unconditionallyClose) + return C.bool(unconditionallyClose) +} diff --git a/v3/pkg/application/webview_window_darwin.go b/v3/pkg/application/webview_window_darwin.go index cf8c5c023..eb3f83769 100644 --- a/v3/pkg/application/webview_window_darwin.go +++ b/v3/pkg/application/webview_window_darwin.go @@ -600,6 +600,7 @@ void windowSetShadow(void* nsWindow, bool hasShadow) { } + // windowClose closes the current window static void windowClose(void *window) { [(WebviewWindow*)window close]; @@ -815,6 +816,7 @@ static void setIgnoreMouseEvents(void *nsWindow, bool ignore) { import "C" import ( "sync" + "sync/atomic" "unsafe" "github.com/wailsapp/wails/v3/internal/assetserver" @@ -903,12 +905,11 @@ func (w *macosWebviewWindow) getScreen() (*Screen, error) { } func (w *macosWebviewWindow) show() { - // macOS implementation is already robust - window container shows immediately - // This is the preferred pattern that Windows should follow C.windowShow(w.nsWindow) } func (w *macosWebviewWindow) hide() { + globalApplication.debug("Window hiding", "windowId", w.parent.id, "title", w.parent.options.Title) C.windowHide(w.nsWindow) } @@ -957,7 +958,11 @@ func (w *macosWebviewWindow) windowZoom() { } func (w *macosWebviewWindow) close() { + globalApplication.debug("Window close() called - setting unconditionallyClose flag", "windowId", w.parent.id, "title", w.parent.options.Title) + // Set the unconditionallyClose flag to allow the window to close + atomic.StoreUint32(&w.parent.unconditionallyClose, 1) C.windowClose(w.nsWindow) + globalApplication.debug("Window close() completed", "windowId", w.parent.id, "title", w.parent.options.Title) // TODO: Check if we need to unregister the window here or not } diff --git a/v3/pkg/application/webview_window_darwin.m b/v3/pkg/application/webview_window_darwin.m index 616d8f4a5..bdd5d8810 100644 --- a/v3/pkg/application/webview_window_darwin.m +++ b/v3/pkg/application/webview_window_darwin.m @@ -7,6 +7,7 @@ extern void processMessage(unsigned int, const char*); extern void processURLRequest(unsigned int, void *); extern void processWindowKeyDownEvent(unsigned int, const char*); extern bool hasListeners(unsigned int); +extern bool windowShouldUnconditionallyClose(unsigned int); @implementation WebviewWindow - (WebviewWindow*) initWithContentRect:(NSRect)contentRect styleMask:(NSUInteger)windowStyle backing:(NSBackingStoreType)bufferingType defer:(BOOL)deferCreation; { @@ -231,6 +232,14 @@ extern bool hasListeners(unsigned int); @implementation WebviewWindowDelegate - (BOOL)windowShouldClose:(NSWindow *)sender { WebviewWindowDelegate* delegate = (WebviewWindowDelegate*)[sender delegate]; + NSLog(@"[DEBUG] windowShouldClose called for window %d", delegate.windowId); + // Check if this window should close unconditionally (called from Close() method) + if (windowShouldUnconditionallyClose(delegate.windowId)) { + NSLog(@"[DEBUG] Window %d closing unconditionally (Close() method called)", delegate.windowId); + return true; + } + // For user-initiated closes, emit WindowClosing event and let the application decide + NSLog(@"[DEBUG] Window %d close requested by user - emitting WindowClosing event", delegate.windowId); processWindowEvent(delegate.windowId, EventWindowShouldClose); return false; } @@ -446,11 +455,13 @@ extern bool hasListeners(unsigned int); } } - (void)windowDidOrderOffScreen:(NSNotification *)notification { + NSLog(@"[DEBUG] Window %d ordered OFF screen (hidden)", self.windowId); if( hasListeners(EventWindowDidOrderOffScreen) ) { processWindowEvent(self.windowId, EventWindowDidOrderOffScreen); } } - (void)windowDidOrderOnScreen:(NSNotification *)notification { + NSLog(@"[DEBUG] Window %d ordered ON screen (shown)", self.windowId); if( hasListeners(EventWindowDidOrderOnScreen) ) { processWindowEvent(self.windowId, EventWindowDidOrderOnScreen); } @@ -526,6 +537,7 @@ extern bool hasListeners(unsigned int); } } - (void)windowWillClose:(NSNotification *)notification { + NSLog(@"[DEBUG] Window %d WILL close (window is actually closing)", self.windowId); if( hasListeners(EventWindowWillClose) ) { processWindowEvent(self.windowId, EventWindowWillClose); } @@ -571,11 +583,13 @@ extern bool hasListeners(unsigned int); } } - (void)windowWillOrderOffScreen:(NSNotification *)notification { + NSLog(@"[DEBUG] Window %d WILL order off screen (about to hide)", self.windowId); if( hasListeners(EventWindowWillOrderOffScreen) ) { processWindowEvent(self.windowId, EventWindowWillOrderOffScreen); } } - (void)windowWillOrderOnScreen:(NSNotification *)notification { + NSLog(@"[DEBUG] Window %d WILL order on screen (about to show)", self.windowId); if( hasListeners(EventWindowWillOrderOnScreen) ) { processWindowEvent(self.windowId, EventWindowWillOrderOnScreen); } diff --git a/v3/pkg/application/webview_window_windows.go b/v3/pkg/application/webview_window_windows.go index ee266f8b9..9a876fd84 100644 --- a/v3/pkg/application/webview_window_windows.go +++ b/v3/pkg/application/webview_window_windows.go @@ -1169,7 +1169,7 @@ func (w *windowsWebviewWindow) WndProc(msg uint32, wparam, lparam uintptr) uintp } case w32.WM_CLOSE: - if w.parent.unconditionallyClose == false { + if atomic.LoadUint32(&w.parent.unconditionallyClose) == 0 { // We were called by `Close()` or pressing the close button on the window w.parent.emit(events.Windows.WindowClosing) return 0 diff --git a/v3/pkg/events/defaults.go b/v3/pkg/events/defaults.go index 926d86a60..376bef36f 100644 --- a/v3/pkg/events/defaults.go +++ b/v3/pkg/events/defaults.go @@ -41,9 +41,6 @@ var defaultWindowEventMapping = map[string]map[WindowEventType]WindowEventType{ Mac.WindowZoomOut: Common.WindowZoomOut, Mac.WindowZoomReset: Common.WindowZoomReset, Mac.WindowShouldClose: Common.WindowClosing, - Mac.WindowDidResignKey: Common.WindowLostFocus, - Mac.WindowDidResignMain: Common.WindowLostFocus, - Mac.WindowDidResize: Common.WindowDidResize, }, "linux": { Linux.WindowDeleteEvent: Common.WindowClosing,