Skip to content

Conversation

@sharwell
Copy link
Contributor

@sharwell sharwell commented Apr 26, 2023

This change builds off observations and instructions provided by @RadwanFaci.

Supersedes #67976

@sharwell sharwell requested a review from a team as a code owner April 26, 2023 16:23
@ghost ghost added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Apr 26, 2023
(int)frameType == (int)__WindowFrameTypeFlags.WINDOWFRAMETYPE_Document)
{
TrackNewActiveWindowFrame(frame);
var runningDocumentTable = ThreadingContext.JoinableTaskFactory.Run(() => GetRunningDocumentTableAsync(ThreadingContext.DisposalToken).AsTask());
Copy link
Member

Choose a reason for hiding this comment

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

Won't this just block here instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

➡️ In general, no. This method returns immediately in all but the first invocation, and the first invocation should come through the other path. In the event it doesn't it'll wait here for the service to be initialized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not especially worried here because in the off chance there is a problem, it'll be an item on the call stack in RPS results.

{
if (docData is IVsTextBuffer bufferAdapter)
{
TextBuffer = _documentTracker._editorAdaptersFactoryService.GetDocumentBuffer(bufferAdapter);
Copy link
Member

Choose a reason for hiding this comment

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

Just want to make sure, is our cleanup code resilient to this not being her

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most of the stack here is null annotated and everything I saw had null checks in place. To outside code, this just shows up as the DocumentId being null, which is also the case any time a non-Roslyn document has focus.

@sharwell sharwell merged commit 5b47c7f into dotnet:main Apr 27, 2023
@sharwell sharwell deleted the async-open branch April 27, 2023 12:59
@ghost ghost added this to the Next milestone Apr 27, 2023
@Cosifne Cosifne modified the milestones: Next, 17.7 P2 May 31, 2023
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.

4 participants