fix incorrect window destroy (#4323)

* 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 <noreply@anthropic.com>

---------

Co-authored-by: Lea Anthony <lea.anthony@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Atterpac 2025-06-17 15:31:55 -06:00 committed by GitHub
commit 0b7d1f4f67
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 52 additions and 9 deletions

View file

@ -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)

View file

@ -0,0 +1,26 @@
//go:build darwin
package application
/*
#include <stdbool.h>
*/
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)
}

View file

@ -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
}

View file

@ -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);
}

View file

@ -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

View file

@ -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,