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

* [ObjCRuntime] Don't double-retain blocks.

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)

* [tests] Fix test builds.

* [monotouch-test] RegistrarTest.BlockCollection: allocate more and wait longer for the GC.

Allocate more objects and wait longer for the GC to run.

Hopefully fixes this problem:

    	[FAIL] RegistrarTest.BlockCollection :   freed blocks
      Expected: greater than 0
      But was:  0

The blocks are freed if we just wait long enough... The problem is that we
don't want to wait very long (makes the tests slow to run), so try to speed
things up by allocating more.

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