Long ago, `ProjectSnapshotManagerDispatcher` was introduced as a
replacement for a lot of code in Razor that synchronized by running on
the UI thread. This abstraction was needed for the language server,
where there isn't a UI thread. However, there's a lot of code that was
refactored to use `ProjectSnapshotManagerDispatcher` that doesn't really
need it. After all the dispatcher is intended for scheduling updates to
the project snapshot manager. So, using it for services that don't
depend on the project snapshot manager is overuse.
This change removes usage of `ProjectSnapshotManagerDispatcher` in most
of the Visual Studio layer. It's still used by the
`ProjectWorkspaceStateGenerator`, but that requires a larger
refactoring. Most services that were using
`ProjectSnapshotManagerDispatcher` did so to control updates to shared
data structures, and made these free-threaded.
Stop building no longer shipping net7 build
With the move for rzls to publish net8 binaries, we no longer use our net7 build. Remove the TFM from our builds (should speed CI builds)
`Microsoft.AspNetCore.Razor.LanguageServer.Protocol` just contains a few
types that can easily be moved to more appropriate locations. This
change removes that project and reduces our assembly count by 1.
We don't get much from using MS.Ext.Logging for our logging, and we pay
for it with ~two~ three (?) DLL loads in VS. This fixes that by
implementing the logging bits outselves, and removing the abstraction
for options monitoring which we weren't really using.
I also removed the notion of "scopes" from the logging because we never
actually implemented them, so this also closes
https://github.com/dotnet/razor/issues/8232. We don't actually do any
structured logging so we're really not missing much.
The only place that does still use MS.Ext.Logging is our one DLL that is
referenced by Roslyn in VS Code, since they pass us an ILogger, so we
don't really have a choice about that bit :)
I still need to do a little more testing of the options bit, ~because
I'm seeing some oddities, but I think the issue is really one with the
unified settings experience and/or our options updating code.~ Okay
yeah, this is happening in VS main too. Logged
https://github.com/dotnet/razor/issues/10200 but will make sure this
doesn't get any worse :)
Not sure whether me or @DustinCampbell broke this, but hit this assert
during debugging today. What was happening was:
* WorkspaceProjectStateChangeDetector tried to create a ProjectKey for a
Project that had no compilation output assembly path
* `ProjectKey` calls `FilePathNormalizer.GetNormalizedDirectoryName`,
passes in `null`, and gets back `/`
* `ProjectKey` then calls `FilePathNormalizer.NormalizeDirectory` on
that `/`, which exploded
This fixes the first bullet point, by checking if Roslyn knows about the
intermediate output path before continuing. Since this is only checking
for project existence, we don't actually need to go further.
This fixes the third point by protecting against a single `/` being
written for an input that is only a single `/`
This ignores the second point because it doesn't really matter, though
it is slightly wasteful to get a normalized directory name, and then
normalize it.
This is an automatically generated pull request from release/dev17.10
into main.
Once all conflicts are resolved and all the tests pass, you are free to
merge the pull request. 🐯
## Troubleshooting conflicts
### Identify authors of changes which introduced merge conflicts
Scroll to the bottom, then for each file containing conflicts copy its
path into the following searches:
- https://github.com/dotnet/razor/find/release/dev17.10
- https://github.com/dotnet/razor/find/main
Usually the most recent change to a file between the two branches is
considered to have introduced the conflicts, but sometimes it will be
necessary to look for the conflicting lines and check the blame in each
branch. Generally the author whose change introduced the conflicts
should pull down this PR, fix the conflicts locally, then push up a
commit resolving the conflicts.
### Resolve merge conflicts using your local repo
Sometimes merge conflicts may be present on GitHub but merging locally
will work without conflicts. This is due to differences between the
merge algorithm used in local git versus the one used by GitHub.
``` bash
git fetch --all
git checkout -t upstream/merges/release/dev17.10-to-main
git reset --hard upstream/main
git merge upstream/release/dev17.10
# Fix merge conflicts
git commit
git push upstream merges/release/dev17.10-to-main --force
```
BackgroundDocumentGenerator uses a `Timer` and a `Dictionary` to process
work in batches. This change replaces that with `AsyncBatchingWorkQueue`
to greatly simplify the implementation.
Back in 17.10p1, the FallbackProjectManager would apply changes to the
project snapshot manager without scheduling the updates on the
dispatcher. This had the potential to create correctness and reliability
problems if those changes collided with other updates that *were*
scheduled on the dispatcher.
To address the correctness problem, I made the FallbackProjectManager
async and awaited any updates to the project snapshot manager. However,
this created re-entrancy problems that resulted in deadlocks with
Roslyn. I fixed a couple of those re-entrancy problems in 17.10p2, but
hangs have persisted.
A recent Watson dump revealed another yet another re-entrancy issue with
background document promotion in the IRazorDynamicFileInfoProvider. The
hang occurs when waiting for the FallbackProjectManager to add the file,
which causes the project snapshot manager to notify listeners of the
update, which causes one of the listener to try and perform the
background document promotion again, which causes the hang.
To address this, I've decided to just "fire-and-forget" the updates to
the project snapshot manager. The updates will still happen on the
dispatcher, but we won't wait for them and shouldn't hang.
Dustin Campbell
Jan Jones
dotnet-maestro[bot]
Jared Parsons
David Wengier
Rainer Sigwald
Phil Allen
Andrew Hall
dotnet bot