Initialize default capabilities before switching to the UI thread#60106
Initialize default capabilities before switching to the UI thread#60106davidwengier merged 3 commits intodotnet:mainfrom
Conversation
src/Features/LanguageServer/Protocol/DefaultCapabilitiesProvider.cs
Outdated
Show resolved
Hide resolved
dibarbet
left a comment
There was a problem hiding this comment.
If the following comment is true, should we just do that?
// The correct fix for this is to fix the threading violations in the option persister code;
// asserting a MEF component is constructed on the foreground thread is never allowed, but alas it's
// done there. Fixing that isn't difficult but comes with some risk I don't want to take for 16.9;
@jasonmalinowski as best as I can tell this is the only persister that uses the UI thread
https://sourceroslyn.io/#Microsoft.VisualStudio.LanguageServices/Implementation/Options/LanguageSettingsPersister.cs,30
What were you thinking as the solution? To me it seems like the persister requires the UI thread to be functional at all, so I wanted to understand how you were thinking of removing that dependency.
That is blocked on platform work if the issue referenced in the comment is to be believed: #29602 (comment) |
…er.cs Co-authored-by: Sam Harwell <sam@tunnelvisionlabs.com>
|
The idea I had was to bypass the platform API that gives us the language settings as a struct and instead use the settings API to read the underlying settings directly with the free-threaded settings API. That absolutely means we're baking in the implementation detail and leaking the abstraction, but should work? |
|
/azp run roslyn-integration-corehost |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Is there any way to frontload this soon? It would be fantastic if we can get the UI thread dep off of options. |
|
That is tracked by #29602 so might be good to discuss that there, so it doesn't get lost. This is just meant to be targeted to remove the UI delay |
|
ping for review @jasonmalinowski @dibarbet @sharwell ? |
jasonmalinowski
left a comment
There was a problem hiding this comment.
This works, but there might be a cleaner approach here. No matter what, I'd appreciate some updates on the comments as requested.
src/Features/LanguageServer/Protocol/DefaultCapabilitiesProvider.cs
Outdated
Show resolved
Hide resolved
…read (dotnet#60106)" This reverts commit 9b879fb.
Fixes AB#1468258