[AppKit] Try to make NSWindow.ReleasedWhenClosed work. Fixes #8606. (#17105)

NSWindow.ReleasedWhenClosed is a rather annoying API, because it interferes with
reference counting. In effect it behaves kind of like autoreleasing does, except
that the object (window) is released at a different time (when the window is closed,
as opposed to when the autorelease pool drains).

We've tried to fix this in the past in several ways:

* Forcefully disable ReleasedWhenClosed in all the constructors, and if it's set
  in the Close method, then add an extra Retain call to offset the imminent extra
  release. The unfortunate side-effect is that we also call Dispose, which might
  be too early (see #8606).

* Rewrite the code to correctly override the native 'close' method, so we get the
  supposedly correct semantics (our special Close code is called) when the window
  is closed by Objective-C (for instance when the user hits the red X to close the
  window). This doesn't really solve the previous problem (we're calling Dispose
  too early), and it doesn't work for non-subclassed NSWindows (see #8717).

So I'm trying another approach: track the value of ReleasedWhenClosed, and call Retain/Release
when the value switches between true/false. This way we don't need any special logic
in the Close method.

I've also:

* Marked the ReleasedWhenClosed property as obsolete, and added a DangerousReleasedWhenClosed
  property, to match how we bind the other reference counting methods (retain ->
  DangerousRetain, release -> DangerousRelease, etc.).

* Added a ReleaseWhenClosed(bool) method, to be called instead of the ReleasedWhenClosed
  property, and this method will call DangerousRetain/DangerousRelease as described
  above when the ReleasedWhenClosed property changes value.

This new tracking behavior is opt-in for now, but will become opt-out in .NET 9,
and hopefully we'll be able to make it the only behavior at some point (in .NET 10
maybe?).

Fixes #8606.
Fixes #8607.

Ref: #8717.

Author: rolfbjarne

src/AppKit/NSWindow.cs

@@ -24,6 +24,8 @@
 #if !__MACCATALYST__
 
 using System;
+using System.ComponentModel;
+
 using CoreFoundation;
 using Foundation;
 using ObjCRuntime;
@@ -32,8 +34,31 @@ namespace AppKit {
 
 	public partial class NSWindow {
 
+		// Automatically set ReleaseWhenClosed=false in every constructor.
+		[Obsolete ("Set 'TrackReleasedWhenClosed' and call 'ReleaseWhenClosed()' instead.")]
+		[EditorBrowsable (EditorBrowsableState.Never)]
 		public static bool DisableReleasedWhenClosedInConstructor;
 
+		// * If 'releasedWhenClosed' is true at the end of the constructor, automatically call 'retain'.
+		// * Enable our own managed 'ReleaseWhenClosed (bool)' method (which will call 'retain'/'release' as needed).
+		// * Don't do anything in Close (most importantly do not call Dispose).
+		// If 'TrackReleasedWhenClosed' is not set, we default to 'false' until .NET 9, and then we default to 'true'.
+		// Hopefully we can then remove the variable and just implement the tracking as the only behavior at some point
+		// (XAMCORE_5_0 && NET10_0?)
+		static bool? track_relased_when_closed;
+		public static bool TrackReleasedWhenClosed {
+			get {
+#if NET9_0
+				return track_relased_when_closed != false;
+#else
+				return track_relased_when_closed == true;
+#endif
+			}
+			set {
+				track_relased_when_closed = value;
+			}
+		}
+
 		static IntPtr selInitWithWindowRef = Selector.GetHandle ("initWithWindowRef:");
 
 		// Do not actually export because NSObjectFlag is not exportable.
@@ -47,25 +72,57 @@ private NSWindow (IntPtr windowRef, NSObjectFlag x) : base (NSObjectFlag.Empty)
 			} else {
 				Handle = ObjCRuntime.Messaging.IntPtr_objc_msgSendSuper (this.SuperHandle, selInitWithWindowRef);
 			}
-			if (!DisableReleasedWhenClosedInConstructor)
-				ReleasedWhenClosed = false;
+			InitializeReleasedWhenClosed ();
 		}
 
 		static public NSWindow FromWindowRef (IntPtr windowRef)
 		{
 			return new NSWindow (windowRef, NSObjectFlag.Empty);
 		}
 
+		void InitializeReleasedWhenClosed ()
+		{
+			if (TrackReleasedWhenClosed) {
+				if (DangerousReleasedWhenClosed)
+					DangerousRetain ();
+			} else if (!DisableReleasedWhenClosedInConstructor) {
+				DangerousReleasedWhenClosed = false;
+			}
+		}
+
+		// Call DangerousRetain when 'ReleasedWhenClosed' changes from false to true.
+		// Call DangerousRelease when 'ReleasedWhenClosed' changes from true to false.
+		public void ReleaseWhenClosed (bool value = true)
+		{
+			if (!TrackReleasedWhenClosed)
+				throw new InvalidOperationException ($"The NSWindow.{nameof (TrackReleasedWhenClosed)} field must be set to 'true' when calling this method.");
+
+			if (DangerousReleasedWhenClosed == value)
+				return;
+
+			if (value) {
+				DangerousRetain ();
+			} else {
+				DangerousRelease ();
+			}
+			DangerousReleasedWhenClosed = value;
+		}
+
 		public void Close ()
 		{
+			if (TrackReleasedWhenClosed) {
+				_Close ();
+				return;
+			}
+
 			// Windows that do not have a WindowController use released_when_closed
 			// if set to true, the call to Close will release the object, and we will
 			// end up with a double free.
 			//
 			// If that is the case, we take a reference first, and to keep the behavior
 			// we call Dispose after that.
 			if (WindowController == null) {
-				bool released_when_closed = ReleasedWhenClosed;
+				bool released_when_closed = DangerousReleasedWhenClosed;
 				if (released_when_closed)
 					CFObject.CFRetain (Handle);
 				_Close ();

src/appkit.cs

@@ -20637,16 +20637,16 @@ partial interface NSWindow : NSAnimatablePropertyContainer, NSUserInterfaceItemI
 		CGRect ContentRectFor (CGRect frameRect);
 
 		[Export ("init")]
-		[PostSnippet ("if (!DisableReleasedWhenClosedInConstructor) { ReleasedWhenClosed = false; }", Optimizable = true)]
+		[PostSnippet ("InitializeReleasedWhenClosed ();", Optimizable = true)]
 		NativeHandle Constructor ();
 
 		[DesignatedInitializer]
 		[Export ("initWithContentRect:styleMask:backing:defer:")]
-		[PostSnippet ("if (!DisableReleasedWhenClosedInConstructor) { ReleasedWhenClosed = false; }", Optimizable = true)]
+		[PostSnippet ("InitializeReleasedWhenClosed ();", Optimizable = true)]
 		NativeHandle Constructor (CGRect contentRect, NSWindowStyle aStyle, NSBackingStore bufferingType, bool deferCreation);
 
 		[Export ("initWithContentRect:styleMask:backing:defer:screen:")]
-		[PostSnippet ("if (!DisableReleasedWhenClosedInConstructor) { ReleasedWhenClosed = false; }", Optimizable = true)]
+		[PostSnippet ("InitializeReleasedWhenClosed ();", Optimizable = true)]
 		NativeHandle Constructor (CGRect contentRect, NSWindowStyle aStyle, NSBackingStore bufferingType, bool deferCreation, NSScreen screen);
 
 		[Export ("title")]
@@ -20801,8 +20801,18 @@ partial interface NSWindow : NSAnimatablePropertyContainer, NSUserInterfaceItemI
 		[Internal, Export ("close")]
 		void _Close ();
 
+#if !XAMCORE_5_0
+		[Obsolete ("Call 'ReleaseWhenClosed ()' instead.")]
 		[Export ("releasedWhenClosed")]
 		bool ReleasedWhenClosed { [Bind ("isReleasedWhenClosed")] get; set; }
+#endif
+
+		// releasedWhenClosed is a variation of sending a delayed 'autorelease', and since
+		// we've bound release/retain/autorelease with a 'Dangerous' prefix, we're adding
+		// one for this property as well.
+		[Sealed]
+		[Export ("releasedWhenClosed")]
+		bool DangerousReleasedWhenClosed { [Bind ("isReleasedWhenClosed")] get; set; }
 
 		[Export ("miniaturize:")]
 		void Miniaturize ([NullAllowed] NSObject sender);