clean up the pending table when task is done

also add a new ETW event, when we have circular dependencies in the graph.

Author: Lifeng Lu

src/Microsoft.VisualStudio.Threading/JoinableTask.cs

@@ -1143,6 +1143,7 @@ private bool TryDequeueSelfOrDependencies(bool onMainThread, ref HashSet<IJoinab
                             if (!foundWork && this.PotentialUnreachableDependents != null)
                             {
                                 JoinableTaskDependencyGraph.RemoveUnreachableDependentItems(this, this.PotentialUnreachableDependents, visitedNodes);
+                                this.PotentialUnreachableDependents = null;
                             }
                         }
 

src/Microsoft.VisualStudio.Threading/JoinableTaskDependencyGraph.cs

@@ -165,7 +165,7 @@ internal static void OnTaskCompleted(IJoinableTaskDependent taskItem)
         {
             Requires.NotNull(taskItem, nameof(taskItem));
             Assumes.True(Monitor.IsEntered(taskItem.JoinableTaskContext.SyncContextLock));
-            taskItem.GetJoinableTaskDependentData().OnTaskCompleted();
+            taskItem.GetJoinableTaskDependentData().OnTaskCompleted(taskItem);
         }
 
         /// <summary>
@@ -224,6 +224,7 @@ internal static HashSet<JoinableTask> GetDependentTasksFromCandidates(IEnumerabl
         /// Computes dependency graph to clean up all potential unreachable dependents items.
         /// </summary>
         /// <param name="syncTask">A thread blocking sychornizing task.</param>
+        /// <param name="allReachableNodes">Returns all reachable nodes in the connected dependency graph, if unreachable dependency is found.</param>
         /// <returns>True if it removes any unreachable items.</returns>
         internal static bool CleanUpPotentialUnreachableDependentItems(JoinableTask syncTask, [MaybeNullWhen(false)] out HashSet<IJoinableTaskDependent>? allReachableNodes)
         {
@@ -234,21 +235,21 @@ internal static bool CleanUpPotentialUnreachableDependentItems(JoinableTask sync
             // To get rid of those loops, if a task still tracks the synchronous task after reducing
             // the reference count, we will calculate the entire reachable tree from the root.  That will
             // tell us the exactly tasks which need track the synchronous task, and we will clean up the rest.
-            HashSet<IJoinableTaskDependent>? unreachableItems = syncTask.PotentialUnreachableDependents;
-            if (unreachableItems is object)
+            HashSet<IJoinableTaskDependent>? possibleUnreachableItems = syncTask.PotentialUnreachableDependents;
+            if (possibleUnreachableItems is object)
             {
                 var reachableNodes = new HashSet<IJoinableTaskDependent>();
                 var syncTaskItem = (IJoinableTaskDependent)syncTask;
 
-                JoinableTaskDependentData.ComputeSelfAndDescendentOrJoinedJobsAndRemainTasks(syncTaskItem, reachableNodes, unreachableItems);
+                JoinableTaskDependentData.ComputeSelfAndDescendentOrJoinedJobsAndRemainTasks(syncTaskItem, reachableNodes, possibleUnreachableItems);
 
                 syncTask.PotentialUnreachableDependents = null;
                 allReachableNodes = reachableNodes;
 
                 // force to remove all invalid items
-                if (unreachableItems.Count > 0)
+                if (possibleUnreachableItems.Count > 0)
                 {
-                    JoinableTaskDependentData.RemoveUnreachableDependentItems(syncTask, unreachableItems, reachableNodes);
+                    JoinableTaskDependentData.RemoveUnreachableDependentItems(syncTask, possibleUnreachableItems, reachableNodes);
 
                     return true;
                 }
@@ -419,6 +420,18 @@ internal static void RemoveDependency(IJoinableTaskDependent parentTaskOrCollect
                                 data.childDependentNodes.Remove(joinChild);
                                 data.RemoveDependingSynchronousTaskFromChild(joinChild);
                                 parentTaskOrCollection.OnDependencyRemoved(joinChild);
+
+                                // A node with no out-going dependency chain cannot be a part of a circular dependency loop.
+                                // JoinableTaskCollection doesn't have a completion event, this logic makes sure it will be removed from a long runnning JTF.Run.
+                                if (data.HasNoChildDependentNode)
+                                {
+                                    DependentSynchronousTask? existingTaskTracking = data.dependingSynchronousTaskTracking;
+                                    while (existingTaskTracking is object)
+                                    {
+                                        existingTaskTracking.SynchronousTask.PotentialUnreachableDependents?.Remove(parentTaskOrCollection);
+                                        existingTaskTracking = existingTaskTracking.Next;
+                                    }
+                                }
                             }
                             else
                             {
@@ -541,6 +554,8 @@ internal static void ComputeSelfAndDescendentOrJoinedJobsAndRemainTasks(IJoinabl
             /// <param name="reachableItems">All reachable items.</param>
             internal static void RemoveUnreachableDependentItems(JoinableTask syncTask, HashSet<IJoinableTaskDependent> unreachableItems, HashSet<IJoinableTaskDependent> reachableItems)
             {
+                ThreadingEventSource.Instance.CircularJoinableTaskDependencyDetected(unreachableItems.Count, reachableItems.Count);
+
                 HashSet<IJoinableTaskDependent>? remainPlaceHold = null;
                 foreach (IJoinableTaskDependent? unreachableItem in unreachableItems)
                 {
@@ -615,7 +630,7 @@ internal bool HasMainThreadSynchronousTaskWaiting(IJoinableTaskDependent taskIte
             /// This is called when this task is completed.
             /// This method is expected to be used with the JTF lock.
             /// </summary>
-            internal void OnTaskCompleted()
+            internal void OnTaskCompleted(IJoinableTaskDependent thisDependentNode)
             {
                 if (this.dependingSynchronousTaskTracking is object)
                 {
@@ -628,6 +643,16 @@ internal void OnTaskCompleted()
                         while (existingTaskTracking is object)
                         {
                             RemoveDependingSynchronousTaskFrom(childrenTasks, existingTaskTracking.SynchronousTask, false);
+
+                            HashSet<IJoinableTaskDependent>? potentialUnreachableDependents = existingTaskTracking.SynchronousTask.PotentialUnreachableDependents;
+                            if (potentialUnreachableDependents != null)
+                            {
+                                if (potentialUnreachableDependents.Remove(thisDependentNode) && potentialUnreachableDependents.Count == 0)
+                                {
+                                    existingTaskTracking.SynchronousTask.PotentialUnreachableDependents = null;
+                                }
+                            }
+
                             existingTaskTracking = existingTaskTracking.Next;
                         }
                     }
@@ -921,7 +946,8 @@ private static void RemoveDependingSynchronousTask(IJoinableTaskDependent taskOr
 
                         if (reachableNodes is null)
                         {
-                            if (removed)
+                            // if a node doesn't have dependencies, it cannot be a part of a dependency circle.
+                            if (removed || taskOrCollection.GetJoinableTaskDependentData().HasNoChildDependentNode)
                             {
                                 if (remainingDependentNodes is object)
                                 {

src/Microsoft.VisualStudio.Threading/ThreadingEventSource.cs

@@ -66,6 +66,11 @@ internal sealed partial class ThreadingEventSource : EventSource
         /// </summary>
         private const int PostExecutionStopEvent = 16;
 
+        /// <summary>
+        /// The event ID for the <see cref="CircularJoinableTaskDependencyDetected(int, int)"/>.
+        /// </summary>
+        private const int CircularJoinableTaskDependencyDetectedEvent = 17;
+
         /// <summary>
         /// Logs an issued lock.
         /// </summary>
@@ -153,6 +158,17 @@ public void PostExecutionStop(int requestId)
             this.WriteEvent(PostExecutionStopEvent, requestId);
         }
 
+        /// <summary>
+        /// Circular JoinableTask dependency detected.
+        /// </summary>
+        /// <param name="initUnreachableCount">Initial count of unreachable nodes.</param>
+        /// <param name="reachableCount">The size of the connected dependency graph.</param>
+        [Event(CircularJoinableTaskDependencyDetectedEvent, Level = EventLevel.Informational)]
+        public void CircularJoinableTaskDependencyDetected(int initUnreachableCount, int reachableCount)
+        {
+            this.WriteEvent(CircularJoinableTaskDependencyDetectedEvent, initUnreachableCount, reachableCount);
+        }
+
         /// <summary>
         /// The names of constants in this class make up the middle term in
         /// the AsyncReaderWriterLock/LockRequest/Issued event name.