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

Create new AsyncServiceProvider instead of using AsyncServiceProvider.GlobalProvider #74084

Merged
merged 1 commit into from
Jun 20, 2024

Conversation

sharwell
Copy link
Member

AsyncServiceProvider.GlobalProvider is no longer an instance of AsyncServiceProvider, so the original code is failing.

….GlobalProvider

AsyncServiceProvider.GlobalProvider is no longer an instance of
AsyncServiceProvider, so the original code is failing.
@sharwell sharwell requested a review from a team as a code owner June 20, 2024 18:41
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Jun 20, 2024
@sharwell sharwell enabled auto-merge June 20, 2024 18:41
@@ -455,7 +456,7 @@ public async Task<ImmutableArray<string>> GetF1KeywordsAsync(CancellationToken c
ErrorHandler.ThrowOnFailure(vsView.GetBuffer(out var textLines));
ErrorHandler.ThrowOnFailure(textLines.GetLanguageServiceID(out var languageServiceGuid));

var languageService = await ((AsyncServiceProvider)AsyncServiceProvider.GlobalProvider).QueryServiceAsync(languageServiceGuid).WithCancellation(cancellationToken);
var languageService = await new AsyncServiceProvider((COMAsyncServiceProvider.IAsyncServiceProvider)AsyncServiceProvider.GlobalProvider).QueryServiceAsync(languageServiceGuid).WithCancellation(cancellationToken);
Copy link
Contributor

Choose a reason for hiding this comment

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

As this is a fix for a downcast that wasn't documented to succeed, maybe your fix should avoid doing the same thing again. I believe the only supported way to get the COM IAsyncServiceProvider is by querying for it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Querying for it is easy enough. Do you know which service interface matches it?

Copy link
Contributor

Choose a reason for hiding this comment

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

There isn't a different service interface for it. So it would look like this:

AsyncServiceProvider.GlobalProvider.GetService<COMAsyncServiceProvider.IAsyncServiceProvider>()

You'll have to test it though. This is all based on my recollection.

Copy link
Member Author

Choose a reason for hiding this comment

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

➡️ I'll submit it as a separate follow-up PR since the current one already passed integration tests.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah that will "work" as the managed version implements the native interface, but the right change is to query it as suggested by Andrew.

Copy link
Member Author

Choose a reason for hiding this comment

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

➡️ Submitted #74100

@sharwell sharwell merged commit 8bdf5c4 into dotnet:main Jun 20, 2024
25 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Jun 20, 2024
@sharwell sharwell deleted the async-provider branch June 20, 2024 21:12
@jjonescz jjonescz modified the milestones: Next, 17.11 P3 Jun 24, 2024
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.

5 participants