Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid deadlock on solution close with linked files #17372

Merged

Conversation

dpoeschl
Copy link
Contributor

@dpoeschl dpoeschl commented Feb 24, 2017

Fixes #17305
Fixes
https://devdiv.visualstudio.com/0bdbc590-a062-4c3f-b0f6-9383f67865ee/_workitems?id=388328&_a=edit

Escrow Template

Customer scenario I'm not exactly sure how to reproduce it, but multiple reports point to the same problem. When closing a solution, we close a linked file, which causes the running document table to tell us about a context change, which deadlocks. I believe it can hang before saving the solution.

Bugs this fixes:

#17305
https://devdiv.visualstudio.com/0bdbc590-a062-4c3f-b0f6-9383f67865ee/_workitems?id=388328&_a=edit

Workarounds, if any: Close all open documents before closing the solution.

Risk: Fairly low. We avoid taking a lock when the lock has already been taken for us.

Performance impact: Essentially none. No allocations or complexity changes.

Is this a regression from a previous update?: Unknown, but probably. I can't repro it, but the reports seem to be on RC4+ builds.

Root cause analysis: I don't know what regressed it. This should have been fixed in #16889, but I missed the linked files case. Shared Projects and linked files are now both handled appropriately.

How did we miss it? I didn't think there were any additional callers of a particular overload of OnDocumentContextUpdated.

How was the bug found? Customer reports and dogfooding.

@dpoeschl
Copy link
Contributor Author

Tagging for review: @dotnet/roslyn-ide
FYI: @Pilchie

@Pilchie Pilchie added this to the 2.0 (RTM) milestone Feb 24, 2017
@CyrusNajmabadi
Copy link
Member

👍

@dpoeschl
Copy link
Contributor Author

Tagging @ManishJayaswal and @srivatsn for escrow consideration.

@dpoeschl
Copy link
Contributor Author

dpoeschl commented Feb 24, 2017

retest windows_release_vs-integration_prtest please
Failure: https://ci.dot.net/job/dotnet_roslyn/job/master/job/windows_release_vs-integration_prtest/374/

@natidea
Copy link
Contributor

natidea commented Feb 27, 2017

@dpoeschl you will need to retarget this to dev15.0.x
/cc @MattGertz I'm adding the pending shiproom tag. This is 388328

@MattGertz
Copy link
Contributor

Groovy.

@dpoeschl
Copy link
Contributor Author

Merging into master, not dev15.0.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deadlock in VS while unloading project at OnDocumentContextUpdated
7 participants