Merge pull request #16889 from dpoeschl/NoCPSSharedProjectDeadlocks

Avoid deadlocks when unloading unloading multitargeted Asp.NET projects, or .NET Standard (CPS) Projects (directly or via Solution close) referencing Shared Projects

src/Workspaces/Core/Portable/Workspace/Workspace.cs

@@ -49,6 +49,27 @@ public abstract partial class Workspace : IDisposable
 
         private Action<string> _testMessageLogger;
 
+        /// <summary>
+        /// <see cref="OnProjectRemoved"/> takes the <see cref="_serializationLock"/>, but can also
+        /// cause Shared Project IVsHierarchys to notify us that their context hierarchy has
+        /// changed while we are still holding the lock. In response to this, we try to set the new
+        /// active context document for open files, which also tries to take the lock, and we 
+        /// deadlock.
+        /// 
+        /// For.NET Framework Projects that reference Shared Projects, two things prevent deadlocks
+        /// when projects unload. During solution close, any Shared Projects are disconnected
+        /// before the projects start to unload, so no IVsHierarchy events are fired. During a
+        /// single project unload, we receive notification of the context hierarchy change before
+        /// the project is unloaded, avoiding any IVsHierarchy events if we tell the shared
+        /// hierarchy to set its context hierarchy to what it already is.
+        /// 
+        /// Neither of these behaviors are safe to rely on with .NET Standard (CPS) projects, so we
+        /// have to prevent the deadlock ourselves. We do this by remembering if we're already in 
+        /// the serialization lock due to project unload, and then not take the lock to update 
+        /// document contexts if so (but continuing to lock if it's not during a project unload).
+        /// </summary>
+        private ThreadLocal<bool> _isProjectUnloading = new ThreadLocal<bool>(() => false);
+
         /// <summary>
         /// Constructs a new workspace instance.
         /// </summary>
@@ -415,15 +436,24 @@ protected internal virtual void OnProjectRemoved(ProjectId projectId)
         {
             using (_serializationLock.DisposableWait())
             {
-                CheckProjectIsInCurrentSolution(projectId);
-                this.CheckProjectCanBeRemoved(projectId);
+                _isProjectUnloading.Value = true;
 
-                var oldSolution = this.CurrentSolution;
+                try
+                {
+                    CheckProjectIsInCurrentSolution(projectId);
+                    this.CheckProjectCanBeRemoved(projectId);
+
+                    var oldSolution = this.CurrentSolution;
 
-                this.ClearProjectData(projectId);
-                var newSolution = this.SetCurrentSolution(oldSolution.RemoveProject(projectId));
+                    this.ClearProjectData(projectId);
+                    var newSolution = this.SetCurrentSolution(oldSolution.RemoveProject(projectId));
 
-                this.RaiseWorkspaceChangedEventAsync(WorkspaceChangeKind.ProjectRemoved, oldSolution, newSolution, projectId);
+                    this.RaiseWorkspaceChangedEventAsync(WorkspaceChangeKind.ProjectRemoved, oldSolution, newSolution, projectId);
+                }
+                finally
+                {
+                    _isProjectUnloading.Value = false;
+                }
             }
         }
 

src/Workspaces/Core/Portable/Workspace/Workspace_Editor.cs

@@ -335,7 +335,14 @@ protected void OnDocumentContextUpdated(DocumentId documentId)
 
             if (container != null)
             {
-                OnDocumentContextUpdated(documentId, container);
+                if (_isProjectUnloading.Value)
+                {
+                    OnDocumentContextUpdated_NoSerializationLock(documentId, container);
+                }
+                else
+                {
+                    OnDocumentContextUpdated(documentId, container);
+                }
             }
         }
 
@@ -346,17 +353,22 @@ internal void OnDocumentContextUpdated(DocumentId documentId, SourceTextContaine
         {
             using (_serializationLock.DisposableWait())
             {
-                DocumentId oldActiveContextDocumentId;
+                OnDocumentContextUpdated_NoSerializationLock(documentId, container);
+            }
+        }
 
-                using (_stateLock.DisposableWait())
-                {
-                    oldActiveContextDocumentId = _bufferToDocumentInCurrentContextMap[container];
-                    _bufferToDocumentInCurrentContextMap[container] = documentId;
-                }
+        internal void OnDocumentContextUpdated_NoSerializationLock(DocumentId documentId, SourceTextContainer container)
+        {
+            DocumentId oldActiveContextDocumentId;
 
-                // fire and forget
-                this.RaiseDocumentActiveContextChangedEventAsync(container, oldActiveContextDocumentId: oldActiveContextDocumentId, newActiveContextDocumentId: documentId);
+            using (_stateLock.DisposableWait())
+            {
+                oldActiveContextDocumentId = _bufferToDocumentInCurrentContextMap[container];
+                _bufferToDocumentInCurrentContextMap[container] = documentId;
             }
+
+            // fire and forget
+            this.RaiseDocumentActiveContextChangedEventAsync(container, oldActiveContextDocumentId: oldActiveContextDocumentId, newActiveContextDocumentId: documentId);
         }
 
         protected void CheckDocumentIsClosed(DocumentId documentId)