[macOS] NSWindows are not released when closed, causing odd zombie window behaviour in some cases.

[mitchmindtree]

In the constructor for Window, the following NSWindow method is called:

window.setReleasedWhenClosed_(NO);

Instead, we rely on the Drop implementation for IdRef to release the NSWindow after Window's drop method is called:

impl Drop for IdRef {
    fn drop(&mut self) {
        if self.0 != nil {
            let _: () = unsafe { msg_send![self.0, release] };
        }
    }
}

In most cases this is fine, as a window is normally dropped immediately after it is closed. However, in some multi-window applications (like the multiwindow.rs example), a Window is not dropped until all Windows are closed. This means a user might have many windows that are "closed" but have not been released.

Documentation on what it means to have a "closed yet not released" window is quite sparse, however after doing some experiments it seems that cocoa does not handle this well. For example, when running the multiwindow.rs example, the following strange behaviour occurs:

1. Click one of the windows to focus it.

2. Close that window via the x button.

3. Swipe sideways to switch to a separate desktop. This causes a Focus(false) event to be produced for the window that was just closed. We shouldn't receive any events for the window that was just closed.

4. Swipe back to the original desktop. This causes a Focus(true) event to be produced for the window that was closed and causes it to reappear (though without its toolbar)!

   

5. Click one of the other windows to focus it.

6. Close the window via the x button.

7. Swipe sideways to a different desktop (again causing a Focus(false) event to be produced for the window we just closed).

8. Swipe back to the original desktop. Not only does the window we just closed reappear, now the first window we closed regains its toolbar again!

   

If we we run this experiment again after changing window.setReleasedWhenClosed_(NO);'s input to YES, it seems to run perfectly without any zombie windows reappearing right until the last window is closed and the example segfaults. The segfault occurs at the msg_send![self.0, release] line in IdRef's Drop implementation, right after the WindowDelegate's Drop implementation is run.

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   libobjc.A.dylib               	0x00007fff94b500dd objc_msgSend + 29
1   multiwindow                   	0x000000010b3582d7 objc::message::send_unverified::h8ed50c57081eaa6e + 247
2   multiwindow                   	0x000000010b365973 _$LT$winit..platform..platform..window..WindowDelegate$u20$as$u20$core..ops..Drop$GT$::drop::hae662d13fb7dd91e + 371
3   multiwindow                   	0x000000010b348ef1 drop::h7b51b724eb21df37 + 17
4   multiwindow                   	0x000000010b346d0b drop_contents::hec4d8e005a947e8b + 43
5   multiwindow                   	0x000000010b349317 drop::hec4d8e005a947e8b + 55
6   multiwindow                   	0x000000010b346dbc _$LT$alloc..arc..Arc$LT$T$GT$$GT$::drop_slow::h0f94cef74d0170ff + 60
7   multiwindow                   	0x000000010b34b557 _$LT$alloc..arc..Arc$LT$T$GT$$u20$as$u20$core..ops..Drop$GT$::drop::h271ae1b3d1f6c5f2 + 103
8   multiwindow                   	0x000000010b349001 drop::h97c53ac061c80c60 + 17
9   multiwindow                   	0x000000010b348c99 drop::h5308067440417e4d + 9
10  multiwindow                   	0x000000010b349039 drop::h989f12a2e99dde1a + 9
11  multiwindow                   	0x000000010b34c515 multiwindow::main::h633556848db6cefd + 389
12  multiwindow                   	0x000000010b38f57b __rust_maybe_catch_panic + 27
13  multiwindow                   	0x000000010b38ee47 std::rt::lang_start::h745bea112c3e5e1a + 391
14  multiwindow                   	0x000000010b34c9fa main + 42
15  libdyld.dylib                 	0x00007fff9098d5c9 start + 1

This crash likely occurs due to attempting to release the NSWindow after it has already been automatically released following Closed.

Possible Solution

Perhaps we could solve this by switching window.setReleasedWhenClosed_(NO);'s input to YES and store a regular cocoa::base::id for the NSWindow rather than an IdRef? It seems like IdRef was added as a method to automatically clean up objects that would normally require the user to manually release them, however in this case the NSWindow could do its own cleanup when closed, so it seems IdRef should not be necessary?

I'll do some more reading and see if I can find any more information before settling on this, but it seems like it might be the best option for now.

[monomadic]

I can confirm this is a large problem with my setup, I'm using winit for vst plugin guis and it causes a bug where you cannot properly close the window. Also if you force a close and switch to another app then back, you get the same 'zombie' window described above.

I get the feeling that winit is just leaking memory through dangling window refs on mac and has been since it was created... is this true?

I tried the possible solution @mitchmindtree suggested above on my own fork, but instead created my own wrapper struct called WindowRef that didn't call retain. It doesn't work. I'm guessing there's some order of release operations not satisfied by winit.

[francesca64]

In most cases this is fine, as a window is normally dropped immediately after it is closed. However, in some multi-window applications (like the multiwindow.rs example), a Window is not dropped until all Windows are closed. This means a user might have many windows that are "closed" but have not been released.

After #476, windows can no longer close without being dropped (barring anything unexpected happening on the OS end). This stops the closed window from receiving events, but I can still get the zombie window to appear.

Changing setReleasedWhenClosed_ to YES fixes that, and it doesn't cause a segfault anymore. I'll see about getting a complete fix for this tacked onto #481.

[francesca64]

Okay, good news! While #476 and #481 don't fix this separately, after rebasing #481 (without any further changes) I can't reproduce this anymore.

[mitchmindtree]

I won't get a chance to test this again myself for a while, but I'm happy for this to be closed if you are confident you can no longer reproduce this @francesca64. Thanks!