Skip to content

Conversation

lotodore
Copy link
Contributor

This patch fixes #669 by doing cleanup work in applicationWillTerminate as documented here: https://developer.apple.com/documentation/appkit/nsapplication/terminate(_:)

gio_onDraw(self.handle);
}
- (void)applicationWillTerminate:(NSNotification *)notification {
gio_onDestroy(self.handle);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is problematic if the NSWindow is later dealloc'ed which also calls gio_onDestroy.

According to the NSWindow.close documentation,

when the application terminates, it sends the close message to all windows in its window list

and

If the window is set to be released when closed, a release message is sent to the object after the current event is completed.

Why is NSApplication.terminate not triggering NSWindow.release and then a DestroyEvent?

@lotodore
Copy link
Contributor Author

According to https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/MemoryMgmt/Articles/mmRules.html#//apple_ref/doc/uid/20000994-BAJHFBGH
"When an application terminates, objects may not be sent a dealloc message. Because the process’s memory is automatically cleared on exit, it is more efficient simply to allow the operating system to clean up resources than to invoke all the memory management methods."
Dealloc message does not occur for me if terminate occurs using the quit button. It would be rather strange if it occurs "on some systems". I could change gio_onDestroy to allow it to be called more than once.

@eliasnaur
Copy link
Contributor

Your quoted passage just says that os.Exit and equivalent doesn't run deallocators. Unlike Go finalizers, Objective-C deallocation is deterministic and happens exactly as an object's reference count reaches zero.

My question is: since NSApplication.terminate calls close, why is the NSWindow and in turn the GioView not released? If I patch in

index 1b94b8f93c..1ed294a420 100644
--- a/app/os_macos.m
+++ b/app/os_macos.m
@@ -19,6 +19,15 @@
 @property BOOL presentWithTrans;
 @end

+@interface GioWindow : NSWindow
+@end
+
+@implementation GioWindow
+- (void)dealloc {
+       NSLog(@"window dealloc");
+}
+@end
+
 @implementation GioWindowDelegate
 - (void)windowWillMiniaturize:(NSNotification *)notification {
        NSWindow *window = (NSWindow *)[notification object];
@@ -60,6 +69,11 @@
                gio_onFocus(view.handle, 0);
        }
 }
+- (void) windowWillClose:(NSNotification *) notification {
+       NSLog(@"CLOSE!");
+       NSWindow *window = (NSWindow *)[notification object];
+       window.contentView = nil;
+}
 @end

 static void handleMouse(GioView *view, NSEvent *event, int typ, CGFloat dx, CGFloat dy) {
@@ -377,7 +391,7 @@
                        NSMiniaturizableWindowMask |
                        NSClosableWindowMask;

-               NSWindow* window = [[NSWindow alloc] initWithContentRect:rect
+               NSWindow* window = [[GioWindow alloc] initWithContentRect:rect
                                                                                                           styleMask:styleMask
                                                                                                                 backing:NSBackingStoreBuffered
                                                                                                                   defer:NO];

I get a "CLOSE", but not (NSWindow) "dealloc" when Cmd-Q'ing a program. Why? It's possible that some internal NSApplication.terminate logic only runs close, but never releases the NSWindow to speed up application exits.

I could change gio_onDestroy to allow it to be called more than once.

The lifetime of the GioView and its Go peer is carefully bound through GioView's dealloc, and I'd rather not special case that. It seems to me the right fix is to ensure the NSWindow and in turn the GioView is properly released on terminate.

All this complexity warrants the higher-level question: what do you need the DestroyEvent at exit for? In a crash-only sense, anything you do at exit should have been done at some earlier checkpoint.

@lotodore
Copy link
Contributor Author

The section in the documentation states that dealloc does not (may not) occur on terminate. This is why dealloc is not called in this case. I see why you do not agree with macOS behaviour here, but there is not much I can do in this case.

I use DestroyEvent to save global UI state, so that the application opens in the same state as it was closed. If UI state is not saved in a crash, this is not a big deal. But I don't think it is a good idea to save UI state every time it changes. And since DestroyEvent is provided, I think it should also be called when selecting the Quit menu item.

@eliasnaur
Copy link
Contributor

The section in the documentation states that dealloc does not (may not) occur on terminate. This is why dealloc is not called in this case.

I think the dealloc documentation is a red herring. To me, it's equivalent to Go's finalizers not running on exit. In other words, it counters to naïve expectation that since all memory is implicitly freed on exit, all deallocs should run.

We have a more particular case: Cmd-Q invokes NSApplication.terminate, which invokes NSWindow.close, which under normal circumstances releases the NSWindow, leading to a reference count of 0, which leads to a reference count of 0 on our GioView, which then leads to GioView.dealloc and our DestroyEvent.

This doesn't happen, which leads me to conclude that terminate exits before the reference count decrement. There may also be a memory leak that prevents the ref counts going to 0, but that's unlikely since it all works under normal circumstances.

I use DestroyEvent to save global UI state, so that the application opens in the same state as it was closed. If UI state is not saved in a crash, this is not a big deal. But I don't think it is a good idea to save UI state every time it changes. And since DestroyEvent is provided, I think it should also be called when selecting the Quit menu item.

I believe persistent UI state should be saved every time it changes, in general. It's good to make programs robust to crashes in a "eat your vegetables" kind of way.

However, that's not helpful to you, and I do agree a DestroyEvent on terminate is more consistent than no event. My issue with your current PR is that calling gio_onDestroy on window closing leaves the GioView alive (a bit) whereas the Go peer is destroyed. I moved DestroyEvent to GioView.dealloc exactly to avoid this intermediate state and hard to debug edge cases.

In order to generate a DestroyEvent during terminate, I see two options:

  • Somehow ensure that the GioView is released so that its dealloc is called. I attempted that in my hack above through the window.contentView = nil; line, but that didn't seem to work.
  • Implement our own terminate that mimics NSApplication.terminate yet calls stop instead of exiting. That's a more involved change, but has the nice property that Cmd-Q will make your app.Main call return and allow you to do cleanup work in your main function.

@lotodore
Copy link
Contributor Author

I tried your second suggestion, by calling stop and canceling termination. However, calling stop at this point still suppresses dealloc.
(Added to @implementation GioAppDelegate)

- (NSApplicationTerminateReply)applicationShouldTerminate:(NSApplication *)app {
	[app stop:nil];
	return NSTerminateCancel;
}

If I remove the [app stop:nil] nothing will happen when clicking the quit menu item, but I suppose that is also not helpful.

@eliasnaur
Copy link
Contributor

eliasnaur commented Oct 16, 2025

applicationShouldTerminate merely asks whether termination should proceed, and doesn't close windows if not.
Option 2 is about copying NSApplication.terminate verbatim, but replace its final os.Exit with NSApplication.stop. I don't think it's a realistic option, because if Gio is not in charge of the menu bar we can't ensure that our terminate is called.

That leaves option 1, I'm afraid.

@lotodore
Copy link
Contributor Author

lotodore commented Oct 16, 2025

What I was trying to do was to abort terminate while running stop, which could have been kind of like option 2, but it does not work.
As stated, according to Apple documentation I do not think that option 1 is possible, because dealloc is not called on terminate. I cannot find any way to enforce dealloc on terminate, I think this is by design by macOS. Therefore, I still think my pull request is the best possible workaround for this issue.

@eliasnaur
Copy link
Contributor

I can't take your patch as is. It does work in your case, but not in every other case. What about

  • It's possible that in future you will be able to embed a GioView in a user-provided NSWindow. You can't generate a DestroyEvent until you're sure the GioView is dead and not re-used in a different window.
  • Operating systems that don't terminate their programs (iOS, Android)?

If you don't want to checkpoint your UI state a every change, consider the more portable ConfigEvent and its Config.Focused field. Whenever that turns false, the user is no longer interacting with your window, and it's a good time to checkpoint. ConfigEvent seems to be delivered on Cmd-Q.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants