Skip to content

Conversation

@ToddGrun
Copy link
Contributor

@ToddGrun ToddGrun commented Apr 6, 2025

This PR restores behavior introduced in #77768 that were reverted in PR #77983.
The revert occurred as it was flagged for causing ddrit regressions in some of the WinformsVS64.Designer tests. I was unable to reproduce this scenario locally,
but the RPS tests are able to reliably hit this issue.

Insertion PR demonstrating no regressions in WinformsVS64.Designer DDRIT tests.

Digging into it a bit indicated that creating the OpenTextBufferProvider on a bg thread somehow created a thread affinity that caused
microsoft.visualstudio.shell.design.ni!DocData.CreateNativeDocData's call to msenv!CEditorManager::RegisterInvisibleEditor's to require an RPC main->bg thread switch.
This breaks the assumptions of RegisterInvisibleEditor and weird things start happening.

Instead, we explicitly create the OpenTextBufferProvider on the main thread.

Compare by commit:

Commit 1:
Restore changes from PR #77768 that were reverted in PR #77983

Commit 2:
1) Force OpenTextBufferProvider to be created on main thread
2) Move MiscellaneousFilesWorkspace to still be on bgthread, but ensure it occurs after OpenTextBufferProvider construction
3) Remove a misleading comment from OpenTextBufferProvider.ctor and add a verification that it's called on the main thread
4) Change CheckForExistingOpenDocumentsAsync to yield the thread so it doesn't block the caller if the work could have been done synchronously.

ToddGrun added 2 commits April 6, 2025 10:20
2) Move MiscellaneousFilesWorkspace to still be on bgthread, but ensure it occurs after OpenTextBufferProvider construction
3) Remove a misleading comment from OpenTextBufferProvider.ctor and add a verification that it's called on the main thread
4) Change CheckForExistingOpenDocumentsAsync to yield the thread so it doesn't block the caller if the work could have been done synchronously.
@ghost ghost added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Apr 6, 2025
@ToddGrun ToddGrun changed the title *** FOR TEST PURPOSES ONLY *** Move MiscellaneousFilesWorkspace construction to background thread Apr 7, 2025
@ToddGrun ToddGrun marked this pull request as ready for review April 7, 2025 00:47
@ToddGrun ToddGrun requested a review from a team as a code owner April 7, 2025 00:47
[Import(typeof(SVsServiceProvider))] IServiceProvider serviceProvider,
IAsynchronousOperationListenerProvider listenerProvider)
{
// This ctor does RDT operations, and thus requires the UI thread.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe make some comment here for whoever gets to delete this, to add some comment back over the RDT GetService so people know what's allowed again. And I guess update the version number in the comment accordingly. 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments added.

@ToddGrun
Copy link
Contributor Author

ToddGrun commented Apr 8, 2025

Superseded by #78051

@ToddGrun ToddGrun closed this Apr 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants