[CoreCLR] Rework how we track the lifetime of managed NSObjects. (#14785)

* We now create a tracking GCHandle for all NSObjects, not only the toggled ones.
  CoreCLR will notify us when a tracked GCHandle's target enters finalization, and
  we need to be notified for all NSObjects, not just the toggled ones.
* Augment the tracking callback to know about non-toggled objects, and in that
  case report that the tracking GCHandle is a weak GCHandle.
* There's no need to store the tracking GCHandle in a field in the NSObject instance,
  since we store it in our runtime object_map.
* Remove one place where we set the InFinalizerQueue flag, since it's no longer
  required there (this reverts a previous attempt at fixing this problem - 0622ae4af2)
  - we only set the InFinalizerQueue flag in the xamarin_coreclr_reference_tracking_tracked_object_entered_finalization
  callback now.
* Update a few comments accordingly.

Partial fix for https://github.com/xamarin/xamarin-macios/issues/13531.

Fixes https://github.com/xamarin/xamarin-macios/issues/13921 (again).

runtime/coreclr-bridge.m

@@ -181,6 +181,10 @@ monoobject_dict_free_value (CFAllocatorRef allocator, const void *value)
  *       (NSObjectFlagsInFinalizerQueue), which we fetch in managed code in
  *       the Flags getter.
  *
+ *    Note: we call ObjectiveCMarshal.CreateReferenceTrackingHandle for all
+ *    NSObjects, not only toggled ones, because we need point 5) below to
+ *    happen for all NSObjects, not just toggled ones.
+ *
  * 4) The CoreCLR GC will invoke a callback we installed when calling
  *    ObjectiveCMarshal.Initialize to check if that toggled managed object can
  *    be collected or not. This callback is executed during the GC, which
@@ -194,7 +198,7 @@ monoobject_dict_free_value (CFAllocatorRef allocator, const void *value)
  *    to let us know, and we'll set the corresponding flag in the flags
  *
  * 6) Finally, the GCHandle we got in step 3) is freed when the managed peer
- *    is freed.
+ *    is freed and removed from our object map.
  *
  * Caveat: we don't support the server GC (because it uses multiple threads,
  * and thus may call xamarin_coreclr_reference_tracking_begin_end_callback
@@ -292,26 +296,32 @@ xamarin_coreclr_reference_tracking_is_referenced_callback (void* ptr)
 	int rv = 0;
 	struct TrackedObjectInfo *info = (struct TrackedObjectInfo *) ptr;
 	enum NSObjectFlags flags = info->flags;
+	bool isRegisteredToggleRef = (flags & NSObjectFlagsRegisteredToggleRef) == NSObjectFlagsRegisteredToggleRef;
 	id handle = info->handle;
-	MonoToggleRefStatus res;
+	MonoToggleRefStatus res = (MonoToggleRefStatus) 0;
 
-	res = xamarin_gc_toggleref_callback (flags, handle, NULL, NULL);
+	if (isRegisteredToggleRef) {
+		res = xamarin_gc_toggleref_callback (flags, handle, NULL, NULL);
 
-	switch (res) {
-	case MONO_TOGGLE_REF_DROP:
-		// There's no equivalent to DROP in CoreCLR, so just treat it as weak.
-	case MONO_TOGGLE_REF_WEAK:
+		switch (res) {
+		case MONO_TOGGLE_REF_DROP:
+			// There's no equivalent to DROP in CoreCLR, so just treat it as weak.
+		case MONO_TOGGLE_REF_WEAK:
+			rv = 0;
+			break;
+		case MONO_TOGGLE_REF_STRONG:
+			rv = 1;
+			break;
+		default:
+			LOG_CORECLR (stderr, "%s (%p -> handle: %p flags: %i): INVALID toggle ref value: %i\n", __func__, ptr, handle, flags, res);
+			break;
+		}
+	} else {
+		// If this isn't a toggle ref, it's effectively a weak gchandle
 		rv = 0;
-		break;
-	case MONO_TOGGLE_REF_STRONG:
-		rv = 1;
-		break;
-	default:
-		LOG_CORECLR (stderr, "%s (%p -> handle: %p flags: %i): INVALID toggle ref value: %i\n", __func__, ptr, handle, flags, res);
-		break;
 	}
 
-	LOG_CORECLR (stderr, "%s (%p -> handle: %p flags: %i) => %i (res: %i)\n", __func__, ptr, handle, flags, rv, res);
+	LOG_CORECLR (stderr, "%s (%p -> handle: %p flags: %i) => %i (res: %i) isRegisteredToggleRef: %i\n", __func__, ptr, handle, flags, rv, res, isRegisteredToggleRef);
 
 	return rv;
 }

src/Foundation/NSObject2.cs

@@ -103,11 +103,10 @@ namespace Foundation {
 		// See  "Toggle-ref support for CoreCLR" in coreclr-bridge.m for more information.
 		Flags actual_flags;
 		internal unsafe Runtime.TrackedObjectInfo* tracked_object_info;
-		internal GCHandle? tracked_object_handle;
 
 		unsafe Flags flags {
 			get {
-				// Get back the InFinalizerQueue flag, it's the only flag we'll set in the tracked object info structure.
+				// Get back the InFinalizerQueue flag, it's the only flag we'll set in the tracked object info structure from native code.
 				// The InFinalizerQueue will never be cleared once set, so there's no need to unset it here if it's not set in the tracked_object_info structure.
 				if (tracked_object_info != null && ((tracked_object_info->Flags) & Flags.InFinalizerQueue) == Flags.InFinalizerQueue)
 					actual_flags |= Flags.InFinalizerQueue;
@@ -398,12 +397,6 @@ namespace Foundation {
 			}
 			xamarin_release_managed_ref (handle, user_type);
 			FreeData ();
-#if NET
-			if (tracked_object_handle.HasValue) {
-				tracked_object_handle.Value.Free ();
-				tracked_object_handle = null;
-			}
-#endif
 		}
 
 		static bool IsProtocol (Type type, IntPtr protocol)
@@ -926,20 +919,6 @@ namespace Foundation {
 				if (disposing) {
 					ReleaseManagedRef ();
 				} else {
-#if NET
-					// By adding an external reference to the object from finalizer we will
-					// resurrect it. Since Runtime class tracks the NSObject instances with
-					// GCHandle(..., WeakTrackResurrection) we need to make sure it's aware
-					// that the object was finalized.
-					//
-					// On CoreCLR the non-tracked objects don't get a callback from the
-					// garbage collector when they enter the finalization queue but the
-					// information is necessary for Runtime.TryGetNSObject to work correctly. 
-					// Since we are on the finalizer thread now we can just set the flag
-					// directly here.
-					actual_flags |= Flags.InFinalizerQueue;
-#endif
-
 					NSObject_Disposer.Add (this);
 				}
 			} else {

src/ObjCRuntime/Runtime.CoreCLR.cs

@@ -117,8 +117,7 @@ namespace ObjCRuntime {
 			public NSObject.Flags Flags;
 		}
 
-		// See  "Toggle-ref support for CoreCLR" in coreclr-bridge.m for more information.
-		internal static void RegisterToggleReferenceCoreCLR (NSObject obj, IntPtr handle, bool isCustomType)
+		internal static GCHandle CreateTrackingGCHandle (NSObject obj, IntPtr handle)
 		{
 			var gchandle = ObjectiveCMarshal.CreateReferenceTrackingHandle (obj, out var info);
 
@@ -129,7 +128,19 @@ namespace ObjCRuntime {
 				tracked_info->Handle = handle;
 				tracked_info->Flags = obj.FlagsInternal;
 				obj.tracked_object_info = tracked_info;
-				obj.tracked_object_handle = gchandle;
+
+				log_coreclr ($"GetOrCreateTrackingGCHandle ({obj.GetType ().FullName}, 0x{handle.ToString ("x")}) => Info=0x{((IntPtr) tracked_info).ToString ("x")} Flags={tracked_info->Flags} Created new");
+			}
+
+			return gchandle;
+		}
+
+		// See  "Toggle-ref support for CoreCLR" in coreclr-bridge.m for more information.
+		internal static void RegisterToggleReferenceCoreCLR (NSObject obj, IntPtr handle, bool isCustomType)
+		{
+			unsafe {
+				TrackedObjectInfo* tracked_info = obj.tracked_object_info;
+				tracked_info->Flags = obj.FlagsInternal;
 
 				log_coreclr ($"RegisterToggleReferenceCoreCLR ({obj.GetType ().FullName}, 0x{handle.ToString ("x")}, {isCustomType}) => Info=0x{((IntPtr) tracked_info).ToString ("x")} Flags={tracked_info->Flags}");
 			}

src/ObjCRuntime/Runtime.cs

@@ -1092,7 +1092,17 @@ namespace ObjCRuntime {
 		}
 		
 		internal static void RegisterNSObject (NSObject obj, IntPtr ptr) {
+#if NET
+			GCHandle handle;
+			if (Runtime.IsCoreCLR) {
+				handle = CreateTrackingGCHandle (obj, ptr);
+			} else {
+				handle = GCHandle.Alloc (obj, GCHandleType.WeakTrackResurrection);
+			}
+#else
 			var handle = GCHandle.Alloc (obj, GCHandleType.WeakTrackResurrection);
+#endif
+
 			lock (lock_obj) {
 				if (object_map.Remove (ptr, out var existing))
 					existing.Free ();