IsKeyWindow called on disposed window & Releasing window w/wo default delegate

[aconcagua21]

[When not using window controllers and setting NSWindow.ReleaseWhenClose to true]:

(a) After closing a NSWindow using the Close() method, the IsKeyWindow getter gets called, despite the fact the C# NSWindow instance has already been disposed.

(b) When closing a window using the red close button in the menu bar, this does not happen, but the NSWindow.Dispose() never gets called.

When doing the above after attaching the default window delegate (by adding a c# event handler), then:
(a) -dealloc not sent at all, IsKeyWindow called twice on disposed object.
(b) Neither Dispose() called nor -dealloc sent.

Steps to Reproduce

1. Create a new Cocoa app.
2. Delete the main storyboard, delete the app delegate, remove the storyboard reference from plist.
3. Replace the contents of Main.cs with the code below.
4. Run the app.
5. Use buttons in the window to perform sequences described above.

Expected Behavior

IsKeyWindow should not be called after Close().
Dispose() & dealloc should be always called after closing the window.
Using event handlers should not affect this behavior.

Actual Behavior

As described above

Environment

=== Visual Studio Community 2019 for Mac ===

Version 8.5.5 (build 7)
Installation UUID: b903ca92-227d-4ae3-b9f6-8d05f00abb6e
	GTK+ 2.24.23 (Raleigh theme)
	Xamarin.Mac 6.14.1.39 (d16-5 / 30e8706b4)

	Package version: 608000123

=== Mono Framework MDK ===

Runtime:
	Mono 6.8.0.123 (2019-10/1d0d939dc30) (64-bit)
	Package version: 608000123

=== Roslyn (Language Service) ===

3.5.0-beta4-20125-04+1baa0b3063238ed752ad1f0368b1df6b6901373e

=== NuGet ===

Version: 5.4.0.6315

=== .NET Core SDK ===

SDK: /usr/local/share/dotnet/sdk/3.1.200/Sdks
SDK Versions:
	3.1.200
	3.1.102
	3.1.101
	3.1.100
	3.1.100-preview3-014645
	3.1.100-preview2-014569
	3.0.100
	3.0.100-rc1-014190
	3.0.100-preview8-013656
	2.1.701
	2.1.700
	2.1.505
	2.1.503
	2.1.302
MSBuild SDKs: /Library/Frameworks/Mono.framework/Versions/6.8.0/lib/mono/msbuild/Current/bin/Sdks

=== .NET Core Runtime ===

Runtime: /usr/local/share/dotnet/dotnet
Runtime Versions:
	3.1.2
	3.1.1
	3.1.0
	3.1.0-preview3.19553.2
	3.1.0-preview2.19525.6
	3.0.0
	3.0.0-rc1-19456-20
	3.0.0-preview8-28405-07
	2.1.17
	2.1.16
	2.1.15
	2.1.14
	2.1.13
	2.1.12
	2.1.11
	2.1.9
	2.1.7
	2.1.2

=== Xamarin.Profiler ===

Version: 1.6.12.26
Location: /Applications/Xamarin Profiler.app/Contents/MacOS/Xamarin Profiler

=== Updater ===

Version: 11

=== Apple Developer Tools ===

Xcode 11.4.1 (16137)
Build 11E503a

=== Xamarin Designer ===

Version: 16.5.0.471
Hash: 35aa4889d
Branch: remotes/origin/d16-5
Build date: 2020-02-25 00:52:08 UTC

=== Xamarin.Android ===

Not Installed

=== Microsoft Mobile OpenJDK ===

Java SDK: Not Found

Android Designer EPL code available here:
https://github.com/xamarin/AndroidDesigner.EPL

=== Android SDK Manager ===

Version: 16.5.0.39
Hash: 6fb4c79
Branch: remotes/origin/d16-5
Build date: 2020-04-29 20:09:00 UTC

=== Android Device Manager ===

Version: 16.5.0.73
Hash: dff2f03
Branch: remotes/origin/d16-5
Build date: 2020-04-29 20:09:20 UTC

=== Xamarin Inspector ===

Version: 1.4.3
Hash: db27525
Branch: 1.4-release
Build date: Mon, 09 Jul 2018 21:20:18 GMT
Client compatibility: 1

=== Xamarin.Mac ===

Version: 6.16.0.13 (Visual Studio Community)
Hash: b75deaf82
Branch: d16-5-xcode11.4
Build date: 2020-04-01 21:33:18-0400

=== Xamarin.iOS ===

Xamarin.iOS not installed.
Can't find mtouch or the Version file at /Library/Frameworks/Xamarin.iOS.framework/Versions/Current.

=== Build Information ===

Release ID: 805050007
Git revision: f7b7ac4291b7712b5e02dd65d45685346f321ded
Build date: 2020-04-30 11:54:56-04
Build branch: release-8.5
Xamarin extensions: f7b7ac4291b7712b5e02dd65d45685346f321ded

=== Operating System ===

Mac OS X 10.15.4
Darwin 19.4.0 Darwin Kernel Version 19.4.0
    Wed Mar  4 22:28:40 PST 2020
    root:xnu-6153.101.6~15/RELEASE_X86_64 x86_64

Build Logs

Building IsKeyWindowIssue (Debug)
Build started 5/18/2020 11:40:12 AM.

---

Project "/Users/jirkav/Projects/IsKeyWindowIssue/IsKeyWindowIssue/IsKeyWindowIssue.csproj" (Build target(s)):

Target _BeforeCoreCompileImageAssets:
Directory "obj/Debug/actool" doesn't exist. Skipping.
Target _CoreCompileImageAssets:
Tool /Applications/Xcode.app/Contents/Developer/usr/bin/actool execution started with arguments: --errors --warnings --notices --output-format xml1 --output-partial-info-plist /Users/jirkav/Projects/IsKeyWindowIssue/IsKeyWindowIssue/obj/Debug/actool/partial-info.plist --app-icon AppIcon --minimum-deployment-target 10.15 --platform macosx --compile /Users/jirkav/Projects/IsKeyWindowIssue/IsKeyWindowIssue/obj/Debug/actool/bundle /Users/jirkav/Projects/IsKeyWindowIssue/IsKeyWindowIssue/obj/Debug/actool/cloned-assets/Assets.xcassets
Target _CoreCompileInterfaceDefinitions:
BundleResources Output:
OutputManifests Output:
Target _CoreCompileColladaAssets:
Skipping target "_CoreCompileColladaAssets" because it has no inputs.
Target GenerateTargetFrameworkMonikerAttribute:
Skipping target "GenerateTargetFrameworkMonikerAttribute" because all output files are up-to-date with respect to the input files.
Target CoreCompile:
/Library/Frameworks/Mono.framework/Versions/6.8.0/lib/mono/msbuild/Current/bin/Roslyn/csc.exe /noconfig /nowarn:1701,1702,2008 /nostdlib+ /errorreport:prompt /warn:4 /define:UNIFIED;MACOS;DEBUG /errorendlocation /preferreduilang:en-US /reference:/Library/Frameworks/Xamarin.Mac.framework/Versions/Current/lib/mono/Xamarin.Mac/mscorlib.dll /reference:/Library/Frameworks/Xamarin.Mac.framework/Versions/Current/lib/mono/Xamarin.Mac/System.Core.dll /reference:/Library/Frameworks/Xamarin.Mac.framework/Versions/Current/lib/mono/Xamarin.Mac/System.dll /reference:/Library/Frameworks/Xamarin.Mac.framework/Versions/Current/lib/mono/Xamarin.Mac/Facades/System.Drawing.Common.dll /reference:/Library/Frameworks/Xamarin.Mac.framework/Versions/Current/lib/mono/Xamarin.Mac/Xamarin.Mac.dll /debug+ /debug:portable /optimize- /out:obj/Debug/IsKeyWindowIssue.exe /target:exe /utf8output /langversion:7.3 Main.cs "/var/folders/72/_ft_z9mx1zl61ybkjbpws2mr0000gn/T/Xamarin.Mac,Version=v2.0.AssemblyAttributes.cs"
Target CopyFilesToOutputDirectory:
Copying file from "/Users/jirkav/Projects/IsKeyWindowIssue/IsKeyWindowIssue/obj/Debug/IsKeyWindowIssue.exe" to "/Users/jirkav/Projects/IsKeyWindowIssue/IsKeyWindowIssue/bin/Debug/IsKeyWindowIssue.exe".
IsKeyWindowIssue -> /Users/jirkav/Projects/IsKeyWindowIssue/IsKeyWindowIssue/bin/Debug/IsKeyWindowIssue.exe
Copying file from "/Users/jirkav/Projects/IsKeyWindowIssue/IsKeyWindowIssue/obj/Debug/IsKeyWindowIssue.pdb" to "/Users/jirkav/Projects/IsKeyWindowIssue/IsKeyWindowIssue/bin/Debug/IsKeyWindowIssue.pdb".
Target _DetectSigningIdentity:
Detected signing identity:
Bundle Id: com.emclient.IsKeyWindowIssue
App Id: com.emclient.IsKeyWindowIssue
Target _CopyContentToBundle:
Creating directory '/Users/jirkav/Projects/IsKeyWindowIssue/IsKeyWindowIssue/bin/Debug/IsKeyWindowIssue.app/Contents/Resources'
Copying file from '/Users/jirkav/Projects/IsKeyWindowIssue/IsKeyWindowIssue/obj/Debug/actool/bundle/AppIcon.icns' to '/Users/jirkav/Projects/IsKeyWindowIssue/IsKeyWindowIssue/bin/Debug/IsKeyWindowIssue.app/Contents/Resources/AppIcon.icns'
Copying file from '/Users/jirkav/Projects/IsKeyWindowIssue/IsKeyWindowIssue/obj/Debug/actool/bundle/Assets.car' to '/Users/jirkav/Projects/IsKeyWindowIssue/IsKeyWindowIssue/bin/Debug/IsKeyWindowIssue.app/Contents/Resources/Assets.car'
Target _CompileToNative:
/Library/Frameworks/Xamarin.Mac.framework/Versions/Current/bin/mmp @/Users/jirkav/Projects/IsKeyWindowIssue/IsKeyWindowIssue/obj/Debug/response-file.rsp -q
-q
-q
-q

Build succeeded.
0 Warning(s)
0 Error(s)

Time Elapsed 00:00:11.36

========== Build: 1 succeeded, 0 failed, 0 up-to-date, 0 skipped ==========

Build successful.

Example Project (If Possible)

Main.cs:

using System;
using System.Runtime.InteropServices;
using AppKit;
using CoreGraphics;
using Foundation;

namespace IsKeyWindowIssue
{
	static class Program
	{
		static void Main(string[] args)
		{
			NSApplication.Init();

			NSApplication.SharedApplication.BeginInvokeOnMainThread(() => {
				new MyWindow(new CGRect(100, 100, 220, 260)).MakeKeyAndOrderFront(null);
				new MyWindow(new CGRect(150, 150, 220, 260)).MakeKeyAndOrderFront(null);
				new MyWindow(new CGRect(200, 200, 220, 260)).MakeKeyAndOrderFront(null);
			});

			NSApplication.Main(args);
		}
	}
	
	class MyWindow : NSWindow
	{
		bool disposed;

		public MyWindow(CGRect frame)
		{
			ReleasedWhenClosed = true;
			StyleMask = NSWindowStyle.Titled | NSWindowStyle.Closable;
			SetFrame(frame, false);

			ContentView.AddSubview(new NSButton { Title = "Quit", Action = new ObjCRuntime.Selector("myQuit"), Frame = new CGRect(10, 10, 60, 25) });
			ContentView.AddSubview(new NSButton { Title = "Close", Action = new ObjCRuntime.Selector("myClose"), Frame = new CGRect(10, 40, 60, 25) });
			ContentView.AddSubview(new NSButton { Title = "-Win Delegate", Action = new ObjCRuntime.Selector("removeWinDelegate"), Frame = new CGRect(10, 70, 100, 25) });
			ContentView.AddSubview(new NSButton { Title = "+Win Delegate", Action = new ObjCRuntime.Selector("addWinDelegate"), Frame = new CGRect(10, 100, 100, 25) });
			ContentView.AddSubview(new NSButton { Title = "-App Delegate", Action = new ObjCRuntime.Selector("removeAppDelegate"), Frame = new CGRect(10, 130, 100, 25) });
			ContentView.AddSubview(new NSButton { Title = "+App Delegate", Action = new ObjCRuntime.Selector("addAppDelegate"), Frame = new CGRect(10, 160, 100, 25) });
			ContentView.AddSubview(new NSButton { Title = "GC Collect", Action = new ObjCRuntime.Selector("gcCollect"), Frame = new CGRect(10, 190, 100, 25) });
		}

		[Export("myQuit")]
		void MyQuit()
		{
			Console.WriteLine("MyQuit ENTER");
			NSApplication.SharedApplication.Terminate(this);
			Console.WriteLine("MyQuit LEAVE");
		}

		[Export("myClose")]
		void MyClose()
		{
			Console.WriteLine("MyClose ENTER");
			Close();
			Console.WriteLine("MyClose LEAVE");
		}

		[Export("addAppDelegate")]
		void AddAppDelegate()
		{
			var app = NSApplication.SharedApplication;
			Console.WriteLine($"Prev App Delegate: [{app.Delegate}]");
			if (app.Delegate == null)
			{
				app.DidBecomeActive += (s, e) => { Console.WriteLine("App.DidBcomeActive"); };
			}
			Console.WriteLine($"New App Delegate: [{app.Delegate}]");
		}

		[Export("removeAppDelegate")]
		void RemoveAppDelegate()
		{
			var app = NSApplication.SharedApplication;
			Console.WriteLine($"Prev App Delegate: [{app.Delegate}]");
			if (app.Delegate != null)
			{
				app.DidBecomeActive -= (s, e) => { Console.WriteLine("App.DidBcomeActive"); };
				app.Delegate.Dispose();
				app.Delegate = null;
			}
			Console.WriteLine($"New App Delegate: [{app.Delegate}]");
		}

		[Export("addWinDelegate")]
		void AddWinDelegate()
		{
			Console.WriteLine($"Prev Win Delegate: [{Delegate}]");
			if (Delegate == null)
			{
				this.WillMove += MyWindow_WillMove;
				this.WillClose += MyWindow_WillClose;
			}
			Console.WriteLine($"New Win Delegate: [{Delegate}]");
		}

		[Export("removeWinDelegate")]
		void RemoveWinDelegate()
		{
			Console.WriteLine($"Prev Win Delegate: [{Delegate}]");
			if (Delegate != null)
			{
				this.WillMove -= MyWindow_WillMove;
				this.WillClose -= MyWindow_WillClose;
				Delegate.Dispose();
				Delegate = null;
			}
			Console.WriteLine($"New Win Delegate: [{Delegate}]");
		}

		[Export("gcCollect")]
		void GCCollect()
		{
			Console.WriteLine($"GCCollect ENTER");
			if (Delegate != null)
			{
				GC.Collect();
				GC.WaitForPendingFinalizers();
			}
			Console.WriteLine($"GCCollect LEAVE");
		}

		private void MyWindow_WillClose(object sender, EventArgs e)
		{
			Console.WriteLine("WillClose");
		}

		private void MyWindow_WillMove(object sender, EventArgs e)
		{
			Console.WriteLine("WillMove");
		}

		protected override void Dispose(bool disposing)
		{
			Console.WriteLine($"Dispose({disposing}) ENTER");
			if (!disposed)
			{
				disposed = true;
				base.Dispose(disposing);
			}
			Console.WriteLine($"Dispose LEAVE");
		}

		~MyWindow()
		{
			Dispose(false);
		}

		public override bool IsKeyWindow
		{
			get
			{
				if (disposed)
				{
					Console.WriteLine($"IsKeyWindow on disposed object!");
					Console.WriteLine(String.Join("\n\t", NSThread.NativeCallStack));
					return false;
				}

				return base.IsKeyWindow;
			}
		}

		[Export("dealloc")]
		void Dealloc()
		{
			Console.WriteLine($"dealloc ENTER");
			void_objc_msgSendSuper(SuperHandle, ObjCRuntime.Selector.GetHandle("dealloc"));
			Console.WriteLine($"dealloc LEAVE");
		}

		[DllImport("/usr/lib/libobjc.dylib", EntryPoint = "objc_msgSendSuper")]
		public static extern void void_objc_msgSendSuper(IntPtr receiver, IntPtr selector);
	}
}

IsKeyWindowIssue.zip

[chamons]

This is not a Xamarin bug. You can reproduce the crash you are seeing with a small Obj-C app.

https://gist.github.com/chamons/77d4f75c98274c0cb186068d9e2f5b20

If you swap releasedWhenClosed to true and close, you will crash in the same exact way.

	ReleasedWhenClosed = true;

ReleasedWhenClosed is never safe to use in Xamarin.Mac, full stop. We even have special code in the constructors to disable it from storyboard/nibs.

What it does, in a nutshell, is deallocate live objects underneath us without warning the moment you close them. This is bad, because all of the reasons you hit, and plenty others.

When I set it to false, your example works for me for the limited amount I tested.

[aconcagua21]

Well, than who and when disposes the window? - Nobody and never. Until the dealloc message arrives, you cannot dispose the window, because you would get exactly into the same situation as described. And you never know when the delloc arrives. It can, but it doesn't have to be soon after close message.

[aconcagua21]

As of the crash:
What you saw in your ObjC app was over-release crash, caused by not compensating releaseWhenClosed by an extra retain (that's what NSWindow.Close() method does just before sending the native close message). See fixed ObjC version below - no crash, dealloc received.

The problem is not that the runtime deallocates the native object "the moment you close it", but that it does NOT deallocate it the moment you close it. The runtime deallocates it "later", which is a problem, because the managed object is already disposed. And during (and/or before) the deallocation, it calls IsKeyWindow. The runtime can not deallocate the window immediately because of references in notifications, focus ring management and other stuff.

Anyway, it seems to me that setting ReleaseWhenClose to true is not a solution, because it causes leaking both native and manged NSWindow.

ObjcApp.zip

[chamons]

I'm not sure just adding to the windows NSMutableArray is correct. If you add this code:

    [NSTimer scheduledTimerWithTimeInterval:1 repeats:YES block: ^(NSTimer * timer){
        NSLog (@"%ld", [windows count]);
        NSLog (@"%@", windows[0]);
        NSLog (@"%@", windows[1]);
        NSLog (@"%@", windows[2]);
    }];

You can see that after you close a window:

• the array now had a stale (bad) pointer to a window
• But the count will be 3
• And you'll shortly afterwards crash

Now, you could have close go into your global list and remove it, but at that point what does releasedWhenClosed buy you? I'm not seeing your point, just from the native side.

[aconcagua21]

ReleasedWhenClosed=true just tells Cocoa to send an extra relase "when everything is ready" - after closing the window. Therefore, we have to provide an "extra" retain. But with ARC, I cannot send the retain message. That's why I put the window into the array - I just wanted to demonstrate that the crash you got was not the same one that I got in the first example, and that you cannot use it as a proof that it works the same in ObjC. You're right that the pointer is then bad. But nothing can happen, because I never remove items from the array.

I'm really not sure if you see my point: That the problem is not the ReleasedWhenClosed property and it's value, but the mechanism of disposing/deallocating managed/native NSWindow.

You simply must not Dispose the managed NSWindow in the Close() method. Because the native NSWindow will live longer and will receive messages, which can result in calling the disposed managed window.

And if you don't call Close() but use the red semaphore button, than you leak both objects, regardless of what the value of the property was.

[aconcagua21]

This is what can be done (tested):

Always close the window this way:

1. Send the window the "retain" message if ReleasedWhenClose is true.
2. Add the window to a static "ClosedWindows" list (List).
3. Send the window the "close" message.

And in some "Idle" method:
Remove all windows that have retain count 1 from the ClosedWindows list.

Then there are no leaks and no calls to the disposed object and both objects are disposed/deallocated as soon as possible.

[spouliot]

That issue is bit confusing since it covers related but different things. It might be better to split it if I did not get it right.

IsKeyWindow should not be called after Close().

I agree and we don't call it. However Apple does :(

If this is like iOS (and I have strong reason to believe it is) this happens because any [NS|UI]Window that's closed has to "pass" the key window reponsability to another [NS|UI]Window instance.

As such checking if the current [NS|UI]Window is the key window is needed, hence avery late call. Could it be made earlier ? I would think so, but it's not.

Dispose() & dealloc should be always called after closing the window.

This is about the red/close button from the window chrome, right ?

It's not calling the managed Close but the native close. That's due to how the additional handling code in the managed Close that was added (back in 2012). It makes the OS calls the internal _Close method (which public Close calls). This means the additional logic inside Close is skipped when the action comes from the OS.

With a local fix I get the following using the chrome to close a window

MyClose ENTER
Dispose(True) ENTER
Dispose LEAVE
MyClose LEAVE
140271941637072 IsKeyWindow on disposed object! 
0   IsKeyWindowIssue                    0x000000010018ecc9 xamarin_dyn_objc_msgSend + 217
	1   ???                                 0x00000001064b5a31 0x0 + 4400568881
	2   ???                                 0x0000000108da058b 0x0 + 4443473291
	3   ???                                 0x00000001064c1ee3 0x0 + 4400619235
	4   ???                                 0x000000010806a10e 0x0 + 4429619470
	5   IsKeyWindowIssue                    0x0000000100283662 mono_jit_runtime_invoke + 2354
	6   IsKeyWindowIssue                    0x00000001003b7e87 mono_runtime_invoke_checked + 135
	7   IsKeyWindowIssue                    0x00000001003bc65e mono_runtime_invoke + 142
	8   IsKeyWindowIssue                    0x000000010018c387 xamarin_invoke_trampoline + 6167
	9   IsKeyWindowIssue                    0x000000010018d949 xamarin_arch_trampoline + 105
	10  IsKeyWindowIssue                    0x000000010018eb4e xamarin_x86_64_common_trampoline + 118
	11  AppKit                              0x00007fff31d6f1c3 -[NSWindow(NSCursorRects) _shouldShowCursorRects] + 22
	12  AppKit                              0x00007fff31e5df0c -[NSWindow _shouldShowResizeCursor] + 57
	13  AppKit                              0x00007fff31e5ddcf -[NSWindow(NSWindowResizing) _edgeResizingCursorUpdate:atLocation:] + 44
	14  AppKit                              0x00007fff31eb831f -[NSWindow(NSEventRouting) _handleMouseDownEvent:isDelayedEvent:] + 5073
	15  AppKit                              0x00007fff31e22a81 -[NSWindow(NSEventRouting) _reallySendEvent:isDelayedEvent:] + 2612
	16  AppKit                              0x00007fff31e21e29 -[NSWindow(NSEventRouting) sendEvent:] + 349
	17  AppKit                              0x00007fff31e201b4 -[NSApplication(NSEvent) sendEvent:] + 352
	18  AppKit                              0x00007fff31c6d21f -[NSApplication run] + 707
	19  AppKit                              0x00007fff31c3eff6 NSApplicationMain + 777
	20  ???                                 0x0000000107cded17 0x0 + 4425903383
	21  ???                                 0x0000000107cde9cb 0x0 + 4425902539
	22  ???                                 0x0000000102ab0d43 0x0 + 4339731779
	23  ???                                 0x0000000102ab0f21 0x0 + 4339732257
	24  IsKeyWindowIssue                    0x0000000100283662 mono_jit_runtime_invoke + 2354
	25  IsKeyWindowIssue                    0x00000001003b7e87 mono_runtime_invoke_checked + 135
	26  IsKeyWindowIssue                    0x00000001003c034e do_exec_main_checked + 94
	27  IsKeyWindowIssue                    0x00000001001d836e mono_jit_exec + 414
	28  IsKeyWindowIssue                    0x00000001001db5d5 mono_main + 9397
	29  IsKeyWindowIssue                    0x000000010018f4dc xamarin_main + 1116
	30  IsKeyWindowIssue                    0x0000000100190324 main + 36
	31  libdyld.dylib                       0x00007fff6e946cc9 start + 1
dealloc ENTER 0
dealloc LEAVE

I think this is what you were expecting, right ?

Using event handlers should not affect this behavior.

What are the steps to repro that ?

I does not matter in many cases but event handlers can make things behave differently, see my old blog post https://spouliot.wordpress.com/2012/03/26/events-vs-objective-c-delegates/