[runtime] Always release blocks on the main thread. Fixes #4130. (#4312)

Fixes https://github.com/xamarin/xamarin-macios/issues/4130.

runtime/runtime.m

@@ -2118,6 +2118,16 @@ get_method_block_wrapper_creator (MonoMethod *method, int par, guint32 *exceptio
 	return res;
 }
 
+void
+xamarin_release_block_on_main_thread (void *obj)
+{
+	if ([NSThread isMainThread]) {
+		_Block_release (obj);
+	} else {
+		dispatch_async_f (dispatch_get_main_queue (), obj, (dispatch_function_t) _Block_release);
+	}
+}
+
 /*
  * Creates a System.Delegate to wrap an Objective-C proxy when surfacing parameters from Objective-C to C#.
  * @method: method where the parameter is found
@@ -2167,7 +2177,7 @@ xamarin_get_delegate_for_block_parameter (MonoMethod *method, guint32 token_ref,
 	MONO_EXIT_GC_SAFE;
 
 	if (block_wrapper_queue == NULL)
-		block_wrapper_queue = mono_gc_reference_queue_new ((void(*)(void*))_Block_release);
+		block_wrapper_queue = mono_gc_reference_queue_new (xamarin_release_block_on_main_thread);
 
 	mono_gc_reference_queue_add (block_wrapper_queue, delegate, nativeBlock);
 	pthread_mutex_unlock (&wrapper_hash_lock);

runtime/xamarin/runtime.h

@@ -200,6 +200,7 @@ void			xamarin_create_classes ();
 const char *	xamarin_skip_encoding_flags (const char *encoding);
 void			xamarin_add_registration_map (struct MTRegistrationMap *map);
 uint32_t		xamarin_find_protocol_wrapper_type (uint32_t token_ref);
+void			xamarin_release_block_on_main_thread (void *obj);
 
 bool			xamarin_has_managed_ref (id self);
 bool			xamarin_has_managed_ref_safe (id self);

src/ObjCRuntime/Runtime.cs

@@ -1594,6 +1594,21 @@ namespace ObjCRuntime {
 
 			throw ErrorHelper.CreateError (8003, "Failed to find the closed generic method '{0}' on the type '{1}'.", open_method.Name, closed_type.FullName);
 		}
+
+		[EditorBrowsable (EditorBrowsableState.Never)]
+#if MONOMAC
+		public static void ReleaseBlockOnMainThread (IntPtr block)
+		{
+			if (release_block_on_main_thread == null)
+				release_block_on_main_thread = LookupInternalFunction<intptr_func> ("xamarin_release_block_on_main_thread");
+			release_block_on_main_thread (block);
+		}
+		delegate void intptr_func (IntPtr block);
+		static intptr_func release_block_on_main_thread;
+#else
+		[DllImport ("__Internal", EntryPoint = "xamarin_release_block_on_main_thread")]
+		public static extern void ReleaseBlockOnMainThread (IntPtr block);
+#endif
 	}
 		
 	internal class IntPtrEqualityComparer : IEqualityComparer<IntPtr>

src/generator.cs

@@ -2514,9 +2514,6 @@ public partial class Generator : IMemberGatherer {
 		print ("");
 		print ("[DllImport (\"/usr/lib/libobjc.dylib\")]");
 		print ("static extern IntPtr _Block_copy (IntPtr ptr);");
-		print ("");
-		print ("[DllImport (\"/usr/lib/libobjc.dylib\")]");
-		print ("static extern void _Block_release (IntPtr ptr);");
 
 		while (trampolines.Count > 0){
 			var queue = trampolines.Values.ToArray ();
@@ -2604,7 +2601,7 @@ public partial class Generator : IMemberGatherer {
 			print_generated_code ();
 			print ("~{0} ()", ti.NativeInvokerName);
 			print ("{"); indent++;
-			print ("_Block_release (blockPtr);", ns.CoreObjCRuntime);
+			print ("Runtime.ReleaseBlockOnMainThread (blockPtr);", ns.CoreObjCRuntime);
 			indent--; print ("}");
 			print ("");
 			print ("[Preserve (Conditional=true)]");

tests/bindings-test/ApiDefinition.cs

@@ -323,6 +323,16 @@ namespace Bindings.Test {
 		[Export ("callOptionalStaticCallback")]
 		void CallOptionalStaticCallback ();
 
+		[Static]
+		[Export ("callAssertMainThreadBlockRelease:")]
+		void CallAssertMainThreadBlockRelease (OuterBlock completionHandler);
+
+		[Export ("assertMainThreadBlockReleaseCallback:")]
+		void AssertMainThreadBlockReleaseCallback (InnerBlock completionHandler);
+
+		[Export ("callAssertMainThreadBlockReleaseCallback")]
+		void CallAssertMainThreadBlockReleaseCallback ();
+
 		[Export ("testFreedBlocks")]
 		void TestFreedBlocks ();
 
@@ -331,6 +341,9 @@ namespace Bindings.Test {
 		int FreedBlockCount { get; }
 	}
 
+	delegate void InnerBlock (int magic_number);
+	delegate void OuterBlock ([BlockCallback] InnerBlock callback);
+
 	[BaseType (typeof (NSObject))]
 	interface EvilDeallocator
 	{

tests/bindings-test/RuntimeTest.cs

@@ -55,5 +55,24 @@ namespace Xamarin.Tests
 				};
 			}
 		}
+
+		[Test]
+		public void MainThreadDeallocationTest ()
+		{
+			ObjCBlockTester.CallAssertMainThreadBlockRelease ((callback) => {
+				callback (42);
+			});
+
+			using (var main_thread_tester = new MainThreadTest ()) {
+				main_thread_tester.CallAssertMainThreadBlockReleaseCallback ();
+			}
+		}
+
+		class MainThreadTest : ObjCBlockTester {
+			public override void AssertMainThreadBlockReleaseCallback (InnerBlock completionHandler)
+			{
+				completionHandler (42);
+			}
+		}
 	}
 }

tests/test-libraries/libtest.h

@@ -167,6 +167,11 @@ typedef unsigned int (^RegistrarTestBlock) (unsigned int magic);
 +(void) callRequiredStaticCallback;
 -(void) callOptionalCallback;
 +(void) callOptionalStaticCallback;
+typedef void (^innerBlock) (int magic_number);
+typedef void (^outerBlock) (innerBlock callback);
++(void) callAssertMainThreadBlockRelease: (outerBlock) completionHandler;
+-(void) callAssertMainThreadBlockReleaseCallback;
+-(void) assertMainThreadBlockReleaseCallback: (innerBlock) completionHandler;
 
 -(void) testFreedBlocks;
 +(int) freedBlockCount;
@@ -183,6 +188,12 @@ typedef unsigned int (^RegistrarTestBlock) (unsigned int magic);
 -(void) dealloc;
 @end
 
+// This object asserts that its dealloc function is called on the main thread
+@interface MainThreadDeallocator : NSObject {
+}
+-(void) dealloc;
+@end
+
 #ifdef __cplusplus
 } /* extern "C" */
 #endif

tests/test-libraries/libtest.m

@@ -599,6 +599,45 @@ static Class _TestClass = NULL;
 	assert (called);
 }
 
++(void) callAssertMainThreadBlockRelease: (outerBlock) completionHandler
+{
+	MainThreadDeallocator *obj = [[MainThreadDeallocator alloc] init];
+	__block bool success = false;
+
+	dispatch_sync (dispatch_get_global_queue (DISPATCH_QUEUE_PRIORITY_DEFAULT, 0ul), ^{
+		completionHandler (^(int magic_number)
+		{
+			assert (magic_number == 42);
+			assert ([NSThread isMainThread]); // This may crash way after the failed test has finished executing.
+			success = obj != NULL; // this captures the 'obj', and it's only freed when the block is freed.
+		});
+    });
+	assert (success);
+    [obj release];
+}
+
+-(void) callAssertMainThreadBlockReleaseCallback
+{
+	MainThreadDeallocator *obj = [[MainThreadDeallocator alloc] init];
+	__block bool success = false;
+
+	dispatch_sync (dispatch_get_global_queue (DISPATCH_QUEUE_PRIORITY_DEFAULT, 0ul), ^{
+		[self assertMainThreadBlockReleaseCallback: ^(int magic_number)
+		{
+			assert (magic_number == 42);
+			assert ([NSThread isMainThread]); // This may crash way after the failed test has finished executing.
+			success = obj != NULL; // this captures the 'obj', and it's only freed when the block is freed.
+		}];
+    });
+	assert (success);
+    [obj release];
+}
+
+-(void) assertMainThreadBlockReleaseCallback: (innerBlock) completionHandler
+{
+	assert (!"THIS FUNCTION SHOULD BE OVERRIDDEN");
+}
+
 -(void) testFreedBlocks
 {
 	FreedNotifier* obj = [[FreedNotifier alloc] init];
@@ -634,4 +673,13 @@ static Class _TestClass = NULL;
 }
 @end
 
+@implementation MainThreadDeallocator : NSObject {
+}
+-(void) dealloc
+{
+	assert ([NSThread isMainThread]);
+	[super dealloc];
+}
+@end
+
 #include "libtest.decompile.m"

tests/test-libraries/rename.h

@@ -6,6 +6,7 @@
 	#define ObjCBlockTester   object_ObjCBlockTester
 	#define FreedNotifier     object_FreedNotifier
 	#define EvilDeallocator   object_EvilDeallocator
+	#define MainThreadDeallocator object_MainThreadDeallocator
 	#define FakeType2         object_FakeType2
 	#define UltimateMachine   object_UltimateMachine
 	#define FrameworkTest     object_FrameworkTest
@@ -73,6 +74,7 @@
 	#define ObjCBlockTester   ar_ObjCBlockTester
 	#define FreedNotifier     ar_FreedNotifier
 	#define EvilDeallocator   ar_EvilDeallocator
+	#define MainThreadDeallocator ar_MainThreadDeallocator
 	#define FakeType2         ar_FakeType2
 	#define UltimateMachine   ar_UltimateMachine
 	#define FrameworkTest     ar_FrameworkTest

tests/xtro-sharpie/common-ObjCRuntime.ignore

@@ -1,5 +1,4 @@
 !unknown-pinvoke! _Block_copy bound
-!unknown-pinvoke! _Block_release bound
 !unknown-pinvoke! class_addIvar bound
 !unknown-pinvoke! class_addMethod bound
 !unknown-pinvoke! class_addProperty bound