From 6d0ac93ba9d24f6d794419ac2abbfd1d23f695c9 Mon Sep 17 00:00:00 2001 From: Jason Malinowski Date: Fri, 15 Feb 2019 14:46:28 -0800 Subject: [PATCH 1/2] Move open/closed document checks under the _serializationLock These not being under the lock would mean parallel calls to OnDocumentOpened/Closed might race against each other. We don't think this is a practical problem as current callers are already taking their own locks or are affinitized to the UI thread, but it would be good to fix nonetheless. --- .../Portable/Workspace/Workspace_Editor.cs | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_Editor.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_Editor.cs index fd2635405a85..c474b0f032be 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace_Editor.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_Editor.cs @@ -372,11 +372,11 @@ protected internal void OnDocumentOpened( DocumentId documentId, SourceTextContainer textContainer, bool isCurrentContext = true) { - CheckDocumentIsInCurrentSolution(documentId); - CheckDocumentIsClosed(documentId); - using (_serializationLock.DisposableWait()) { + CheckDocumentIsInCurrentSolution(documentId); + CheckDocumentIsClosed(documentId); + var oldSolution = this.CurrentSolution; var oldDocument = oldSolution.GetDocument(documentId); var oldDocumentState = oldDocument.State; @@ -487,11 +487,11 @@ private void AddToOpenDocumentMap(DocumentId documentId) protected internal void OnAdditionalDocumentOpened(DocumentId documentId, SourceTextContainer textContainer, bool isCurrentContext = true) { - CheckAdditionalDocumentIsInCurrentSolution(documentId); - CheckDocumentIsClosed(documentId); - using (_serializationLock.DisposableWait()) { + CheckAdditionalDocumentIsInCurrentSolution(documentId); + CheckDocumentIsClosed(documentId); + var oldSolution = this.CurrentSolution; var oldDocument = oldSolution.GetAdditionalDocument(documentId); var oldText = oldDocument.GetTextSynchronously(CancellationToken.None); @@ -527,11 +527,11 @@ protected internal void OnAdditionalDocumentOpened(DocumentId documentId, Source protected internal void OnDocumentClosed(DocumentId documentId, TextLoader reloader, bool updateActiveContext = false) { - this.CheckDocumentIsInCurrentSolution(documentId); - this.CheckDocumentIsOpen(documentId); - using (_serializationLock.DisposableWait()) { + this.CheckDocumentIsInCurrentSolution(documentId); + this.CheckDocumentIsOpen(documentId); + // forget any open document info ClearOpenDocument(documentId); @@ -553,10 +553,10 @@ protected internal void OnDocumentClosed(DocumentId documentId, TextLoader reloa protected internal void OnAdditionalDocumentClosed(DocumentId documentId, TextLoader reloader) { - this.CheckAdditionalDocumentIsInCurrentSolution(documentId); - using (_serializationLock.DisposableWait()) { + this.CheckAdditionalDocumentIsInCurrentSolution(documentId); + // forget any open document info ClearOpenDocument(documentId); From 25feb762daa84c1f918c70476d84817810ef4424 Mon Sep 17 00:00:00 2001 From: Jason Malinowski Date: Tue, 19 Feb 2019 11:25:51 -0800 Subject: [PATCH 2/2] Assert the file is open in Workspace.OnAdditionalDocumentClosed Previously opened additional documents weren't counted as open in Workspace.IsDocumentOpen, but after https://github.com/dotnet/roslyn/pull/33165 we can now properly do this check. --- src/Workspaces/Core/Portable/Workspace/Workspace_Editor.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace_Editor.cs b/src/Workspaces/Core/Portable/Workspace/Workspace_Editor.cs index c474b0f032be..c831a37f786b 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace_Editor.cs +++ b/src/Workspaces/Core/Portable/Workspace/Workspace_Editor.cs @@ -556,6 +556,7 @@ protected internal void OnAdditionalDocumentClosed(DocumentId documentId, TextLo using (_serializationLock.DisposableWait()) { this.CheckAdditionalDocumentIsInCurrentSolution(documentId); + this.CheckDocumentIsOpen(documentId); // forget any open document info ClearOpenDocument(documentId);