[corlib] Better checks for release of parent task

mcs/class/corlib/System.Threading.Tasks/Task.cs

@@ -533,12 +533,8 @@ internal void ChildCompleted (AggregateException childEx)
 				ProcessChildExceptions ();
 				Status = exSlot == null ? TaskStatus.RanToCompletion : TaskStatus.Faulted;
 				ProcessCompleteDelegates ();
-				if (parent != null &&
-#if NET_4_5
-				    !HasFlag (parent.CreationOptions, TaskCreationOptions.DenyChildAttach) &&
-#endif
-					HasFlag (creationOptions, TaskCreationOptions.AttachedToParent))
-					parent.ChildCompleted (this.Exception);
+				if (parent != null && NotifyParentOnFinish ())
+					parent = null;
 			}
 		}
 
@@ -573,22 +569,13 @@ internal void Finish ()
 				wait_handle.Set ();
 
 			// Tell parent that we are finished
-			if (parent != null && HasFlag (creationOptions, TaskCreationOptions.AttachedToParent)) {
-				if(
-#if NET_4_5
-					!HasFlag (parent.CreationOptions, TaskCreationOptions.DenyChildAttach) &&
-#endif
-					status != TaskStatus.WaitingForChildrenToComplete) {
-						parent.ChildCompleted (this.Exception);
-					}
-			}
-			else {
+			if (parent != null && NotifyParentOnFinish ()) {
+				//
 				// Break the reference back to the parent, otherwise any Tasks created from another Task's thread of 
 				// execution will create an undesired linked-list that the GC cannot free. See bug #18398.
 				//
 				parent = null;
 			}
-
 
 			// Completions are already processed when task is canceled or faulted
 			if (status == TaskStatus.RanToCompletion)
@@ -604,6 +591,20 @@ internal void Finish ()
 				cancellationRegistration.Value.Dispose ();
 		}
 
+		bool NotifyParentOnFinish ()
+		{
+			if (!HasFlag (creationOptions, TaskCreationOptions.AttachedToParent))
+				return true;
+#if NET_4_5
+			if (HasFlag (parent.CreationOptions, TaskCreationOptions.DenyChildAttach))
+				return true;
+#endif
+			if (status != TaskStatus.WaitingForChildrenToComplete)
+				parent.ChildCompleted (Exception);
+
+			return false;
+		}
+
 		void ProcessCompleteDelegates ()
 		{
 			if (continuations.HasElements) {

mcs/class/corlib/Test/System.Threading.Tasks/TaskTest.cs

@@ -1926,72 +1926,74 @@ public void LazyCancelationTest ()
 				Assert.That (ex.InnerException, Is.TypeOf (typeof (TaskCanceledException)), "#3");
 			}
 		}
-
-		// Xamarin Bugzilla 18398.  Simulate recursive rearming task continuations that are typical in long-running 
-		// servers, and verify that Tasks are not getting leaked.
-		//
+
 		[Test]
 		public void TaskContinuationChainLeak()
 		{
 			// Start cranking out tasks, starting each new task upon completion of and from inside the prior task.
 			//
-			var tester = new TaskContinuationChainLeakTester();
-			tester.Run();
-
-			// Pile up some tasks.
-			//
-			Thread.Sleep(500);
+			var tester = new TaskContinuationChainLeakTester ();
+			tester.Run ();
+			tester.TasksPilledUp.WaitOne ();
 
 			// Head task should be out of scope by now.  Manually run the GC and expect that it gets collected.
 			// 
-			GC.Collect();
-			GC.WaitForPendingFinalizers();
+			GC.Collect ();
+			GC.WaitForPendingFinalizers ();
 
-			try
-			{
+			try {
 				// It's important that we do the asserting while the task recursion is still going, since that is the 
 				// crux of the problem scenario.
 				//
-				tester.Assert();
-			}
-			finally
-			{
-				tester.Stop();
+				tester.Verify ();
+			} finally {
+				tester.Stop ();
 			}
 		}
 
 		class TaskContinuationChainLeakTester
 		{
 			volatile bool m_bStop;
-
-			WeakReference<Task> m_headTaskWeakRef;
+			int counter;
+			ManualResetEvent mre = new ManualResetEvent (false);
+			WeakReference<Task> headTaskWeakRef;
+
+			public ManualResetEvent TasksPilledUp {
+				get {
+					return mre;
+				}
+			}
 
-			public void Run()
+			public void Run ()
 			{
-				m_headTaskWeakRef = new WeakReference<Task>(StartNewTask());
+				headTaskWeakRef = new WeakReference<Task> (StartNewTask ());
 			}
 
-			public Task StartNewTask()
+			public Task StartNewTask ()
 			{
-				if (m_bStop) return null;
+				if (m_bStop)
+					return null;
+
+				if (++counter == 50)
+					mre.Set ();
 
-				return Task.Factory.StartNew(DummyWorker).ContinueWith(task => StartNewTask());
+				return Task.Factory.StartNew (DummyWorker).ContinueWith (task => StartNewTask ());
 			}
 
-			public void Stop()
+			public void Stop ()
 			{
 				m_bStop = true;
 			}
 
-			public void Assert()
+			public void Verify ()
 			{
-				Task task = null;
-				Assert.IsFalse(m_headTaskWeakRef.TryGetTarget(out task));
+				Task task;
+				Assert.IsFalse (headTaskWeakRef.TryGetTarget (out task));
 			}
 
-			void DummyWorker()
+			void DummyWorker ()
 			{
-				Thread.Sleep(1);
+				Thread.Sleep (0);
 			}
 		}