Remove the cancellation token from the calculation of InProgressState… #75007
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
….LazyStaleCompilationWithGeneratedDocuments (dotnet#75001) * Remove strong the cancellation token used in the calculation of InProgressState.LazyStaleCompilationWithGeneratedDocuments This will hopefully address multiple recent feedback tickets around Go To Definition occasionally failing. The umbrella pri 0 feedback ticket is https://developercommunity.visualstudio.com/t/Go-to-definition-doesnt-work/10729503 I was able to reproduce this by following the repro outlined by Garry in the feedback ticket, using the "Code Search" window and doing go to definition multiple times. The issue here is that RegularCompilationTracker.FinalizeCompilationWorkerAsync tries to get the value from the Lazy inProgressState.LazyStaleCompilationWithGeneratedDocuments. This lazy holds onto an action that uses a CancellationToken that has already been cancelled, whereas the CancellationToken used by FinalizeCompilationWorkerAsync has not been cancelled. When the action for the lazy is performed, the corresponding OperationCanceledException ends up escaping out of FinalizeCompilationWorker due to the FatalError.ReportAndPropagateUnlessCanceled exception clause failing due to the cancellation token passed into FinalizeCompilationWorkerAsync not being in a cancelled state. The offending Lazy is constructed by RegularCompilationTracker.WithDoNotCreateCreationPolicy's call to the InProgressState ctor. Instead of passing a closure capturing a cancellation token, I've changed the code to instead use CancellableLazy, which takes in a cancellation token at the time the value is requested.
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
CyrusNajmabadi
approved these changes
Sep 6, 2024
arunchndr
approved these changes
Sep 6, 2024
This was referenced Oct 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
….LazyStaleCompilationWithGeneratedDocuments (#75001)
This will hopefully address multiple recent feedback tickets around Go To Definition occasionally failing. The umbrella pri 0 feedback ticket is https://developercommunity.visualstudio.com/t/Go-to-definition-doesnt-work/10729503
I was able to reproduce this by following the repro outlined by Garry in the feedback ticket, using the "Code Search" window and doing go to definition multiple times.
The issue here is that RegularCompilationTracker.FinalizeCompilationWorkerAsync tries to get the value from the Lazy inProgressState.LazyStaleCompilationWithGeneratedDocuments. This lazy holds onto an action that uses a CancellationToken that has already been cancelled, whereas the CancellationToken used by FinalizeCompilationWorkerAsync has not been cancelled. When the action for the lazy is performed, the corresponding OperationCanceledException ends up escaping out of FinalizeCompilationWorker due to the FatalError.ReportAndPropagateUnlessCanceled exception clause failing due to the cancellation token passed into FinalizeCompilationWorkerAsync not being in a cancelled state.
The offending Lazy is constructed by RegularCompilationTracker.WithDoNotCreateCreationPolicy's call to the InProgressState ctor. Instead of passing a closure capturing a cancellation token, I've changed the code to instead use CancellableLazy, which takes in a cancellation token at the time the value is requested.