Merge master to dev16.1-preview1 #32468
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The basic mental here is to determine a context for a file, we start with the Running Document Table and grab the IVsHierarchy for that file. We then chase it through shared projects until we arrive at final IVsHierarchy. This should be the IVsHierarchy we were given for the project. In the case of multitargeting projects, there's one last hop where we fetch a VSHPROPID_ActiveIntellisenseProjectContext that just returns the project name instead of a IVsHierarchy. If you squint you could imagine that as if it returned the IVsHierarchy for that project except we don't have unique ones. To set the context, we start with the project and walk those IVsHierarchy pointers backwards. At each step we can do an operation to figure out which IVsHierarchy needs its pointer updated, and we point it to build the chain, moving back to the IVsHierarchy that will end up in the Running Document Table. This is (still) leaking IVsHierarchies that we subscribe to, and that will be fixed next.
We already had this inside of VisualStudioRuleSetManager, and we need it for more things too. For now all the locking semantics are being kept identical.
Because the code wasn't finished, we were leaking IVsHierarchies since we had no cleanup logic. We hold onto the IVsHierarchies per document cookie (although we do share the watchers), so if we close a file we only unhook from the correct IVsHierarchies rather than having to re- look at everything.
The comment implied that custom handling could be done by overriding this but no such overriding is being done, and trying to do so would be very sketchy.
This behavior causes all sorts of problems: removing a file (which can now happen on a background thread) then tries to call into all sorts of UI-affinitized APIs which are unavailable there. Practically, there's no reason we must do this anyways: if you do remove a file in one context, we'll update our internal structures to point to a new context. There's no real harm in that being temporarily out of sync: either we'll pick the right context at random and the user won't notice, or if we pick the wrong one, they can select the one they want and everything will be updated again. Fixes #31019.
This was setting a magic thread local property which tracked if we were currently unloading a project, and if so avoided taking a lock when the context changed. This was somewhat self-inflicted because we were trying to change project contexts in the Visual Studio shell during the middle of unload. We're not doing that anymore, and removing this also allows us to allow project removal off the UI thread.
This was being applied at the caller, a few lines before the actual call. Now it's obvious what it's doing.
The pattern matches all our other On* methods.
The protected method took the state lock, got the container, let go of the state lock, and then acquired all of the locks. This isn't really safe anyways because once all the locks were released the document could have been closed and disconnected entirely.
This is an optimization that if you have 20 or 30 files open, and only one of them came from a shared asset project, and that project is changing context, we don't need to refresh all of the rest. Since the refreshing is on the UI thread, we want that to be quick.
This disables a few tests that have recently turned up as flaky on Mono. - DestructorOverridesNonDestructor: this test is pretty specific to the JIT / GC implementation. It doesn't run on CoreClr already due to the subtle differences. Shouldn't have been run on Mono in the first place. - TestCallMethodsWithLeastCustomModifiers: there appears to be a runtime race condition around how overloads that differ by only `modopt` elements are resolved. Follow up issue mono/mono#12422
Disable a few tests on Mono
…bugs Finish the project context implementation for the free-threaded project system shims
Merge dev16.0-preview2 to master
ghost
merged commit
xoofx
pushed a commit
to stark-lang/stark-roslyn
that referenced
this pull request
Apr 16, 2019
…preview1 Merge master to dev16.1-preview1
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an automatically generated pull request from master into dev16.1-preview1.
git fetch --all git checkout merges/master-to-dev16.1-preview1 git reset --hard upstream/dev16.1-preview1 git merge upstream/master # Fix merge conflicts git commit git push merges/master-to-dev16.1-preview1 --forceOnce all conflicts are resolved and all the tests pass, you are free to merge the pull request.