[ObjCRuntime] Don't double-retain blocks. (#3717) (#3730)

First there was darkness; no blocks were retained.

Then came the light; and all blocks were retained [1]

Forever.

But all that once is, must one day not be,
and thus the light gave way to darkness,
and blocks were only retained as long as need be [2].

But before there was a balance, there was a crossroad.

In some places the light shone forever,
and all blocks were retained.

In other places there was a balance,
and the light shone only as long as needed.

A desire to unify arose.

Alas, it could not be.

It was a bright and sunny day

When a merge failed [3].

And all blocks were retained. Twice.

Once [here][4] and once [there][5].

For many years we could not see.

Until a dark and rainy night,
when an awareness arose.

And the desire to unify the balance could finally be fulfilled.

[1]: 6efca92acb
[2]: a22f877539
[3]: befa0477cf
[4]: 5158a3c001/src/ObjCRuntime/Runtime.cs (L858)
[5]: 5158a3c001/runtime/runtime.m (L2091)

src/ObjCRuntime/Runtime.cs

@@ -855,7 +855,6 @@ namespace ObjCRuntime {
 #endif
 		static Delegate CreateBlockProxy (MethodInfo method, IntPtr block)
 		{
-			NSObject.DangerousRetain (block);
 			return (Delegate) method.Invoke (null, new object [] { block } );
 		}
 

tests/bindings-test/ApiDefinition.cs

@@ -284,6 +284,13 @@ namespace Bindings.Test {
 
 		[Export ("callOptionalCallback")]
 		void CallOptionalCallback ();
+
+		[Export ("testFreedBlocks")]
+		void TestFreedBlocks ();
+
+		[Static]
+		[Export ("freedBlockCount")]
+		int FreedBlockCount { get; }
 	}
 }
 

tests/bindings-test/RegistrarBindingTest.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Threading;
 
 #if __UNIFIED__
 using Foundation;
@@ -63,7 +64,7 @@ namespace Xamarin.BindingTests
 			}
 		}
 
-		class BlockCallbackTester : ObjCBlockTester
+		public class BlockCallbackTester : ObjCBlockTester
 		{
 			public override void ClassCallback (Action<int> completionHandler)
 			{

tests/bindings-test/bindings-test-mac.csproj

@@ -62,6 +62,7 @@
     <Compile Include="..\api-shared\ObjCRuntime\Registrar.cs">
       <Link>Registrar.cs</Link>
     </Compile>
+    <Compile Include="RegistrarBindingTest.cs" />
   </ItemGroup>
   <ItemGroup>
     <None Include="..\..\tests\test-libraries\libtest.m">

tests/monotouch-test/ObjCRuntime/RegistrarTest.cs

@@ -2026,6 +2026,30 @@ namespace MonoTouchFixtures.ObjCRuntime {
 			}
 		}
 
+		[Test]
+		public void BlockCollection ()
+		{
+			Exception ex = null;
+			int initialFreedCount = ObjCBlockTester.FreedBlockCount;
+			var thread = new Thread (() => {
+				try {
+					using (var obj = new Xamarin.BindingTests.RegistrarBindingTest.BlockCallbackTester ()) {
+						for (int i = 0; i < 10000; i++)
+							obj.TestFreedBlocks ();
+					}
+				} catch (Exception e) {
+					ex = e;
+				}
+			});
+			thread.Start ();
+			thread.Join ();
+			GC.Collect ();
+			GC.WaitForPendingFinalizers ();
+			TestRuntime.RunAsync (DateTime.Now.AddSeconds (30), () => { }, () => ObjCBlockTester.FreedBlockCount > initialFreedCount);
+			Assert.IsNull (ex, "No exceptions");
+			Assert.That (ObjCBlockTester.FreedBlockCount, Is.GreaterThan (initialFreedCount), "freed blocks");
+		}
+
 		[Test]
 		public void TestCtors ()
 		{

tests/test-libraries/libtest.h

@@ -1,5 +1,6 @@
 #import <Foundation/Foundation.h>
 #include <simd/simd.h>
+#include <libkern/OSAtomic.h>
 
 #include "rename.h"
 
@@ -149,6 +150,14 @@ typedef unsigned int (^RegistrarTestBlock) (unsigned int magic);
 -(void) callClassCallback;
 -(void) callRequiredCallback;
 -(void) callOptionalCallback;
+
+-(void) testFreedBlocks;
++(int) freedBlockCount;
+@end
+
+@interface FreedNotifier : NSObject {
+}
+-(void) dealloc;
 @end
 
 #ifdef __cplusplus

tests/test-libraries/libtest.m

@@ -510,6 +510,8 @@ static UltimateMachine *shared;
 }
 @end
 
+static volatile int freed_blocks = 0;
+
 @implementation ObjCBlockTester
 -(void) classCallback: (void (^)(int32_t magic_number))completionHandler
 {
@@ -549,6 +551,30 @@ static UltimateMachine *shared;
 	assert (called);
 }
 
+-(void) testFreedBlocks
+{
+	FreedNotifier* obj = [[FreedNotifier alloc] init];
+	__block bool success = false;
+	[self classCallback: ^(int magic_number)
+	{
+		assert (magic_number == 42);
+		success = obj != NULL; // this captures the 'obj', and it's only freed when the block is freed.
+	}];
+	assert (success);
+	[obj release];
+}
++(int) freedBlockCount
+{
+	return freed_blocks;
+}
+@end
+
+@implementation FreedNotifier
+-(void) dealloc
+{
+	OSAtomicIncrement32 (&freed_blocks);
+	[super dealloc];
+}
 @end
 
 #include "libtest.decompile.m"

tests/test-libraries/rename.h

@@ -4,6 +4,7 @@
 	#define useZLib           object_useZLib
 	#define ObjCRegistrarTest object_ObjCRegistrarTest
 	#define ObjCBlockTester   object_ObjCBlockTester
+	#define FreedNotifier     object_FreedNotifier
 	#define FakeType2         object_FakeType2
 	#define UltimateMachine   object_UltimateMachine
 	#define FrameworkTest     object_FrameworkTest
@@ -67,6 +68,7 @@
 	#define useZLib           ar_useZLib
 	#define ObjCRegistrarTest ar_ObjCRegistrarTest
 	#define ObjCBlockTester   ar_ObjCBlockTester
+	#define FreedNotifier     ar_FreedNotifier
 	#define FakeType2         ar_FakeType2
 	#define UltimateMachine   ar_UltimateMachine
 	#define FrameworkTest     ar_FrameworkTest