Ensure rename cancellation is synchronous #74948
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
| @@ -44,7 +44,8 @@ internal InlineRenameSessionInfo(IInlineRenameSession session) | |||
| internal interface IInlineRenameSession | |||
| { | |||
| /// <summary> | |||
| /// Cancels the rename session, and undoes any edits that had been performed by the session. | |||
| /// Cancels the rename session, and undoes any edits that had been performed by the session. Must be called on the | |||
| /// UI thread. | |||
There was a problem hiding this comment.
this UI affinity was already asserted in the impl.
| await finalCommitAction().ConfigureAwait(true); | ||
| } | ||
| if (finalSolution != null) | ||
| await ApplyChangesAsync(finalSolution).ConfigureAwait(true); |
There was a problem hiding this comment.
this function is now only async for the 'commit' codepath, for teh 'cancel' codepath, it is entirely synchronous.
| @@ -728,6 +737,19 @@ void DismissUIAndRollbackEdits() | |||
|
|
|||
| RenameService.ActiveSession = null; | |||
| } | |||
|
|
|||
| async Task ApplyChangesAsync(Solution finalSolution) | |||
| { | ||
| // ConfigureAwait(true) so we come back to the UI thread to finish work. | ||
| await finalCommitAction().ConfigureAwait(true); | ||
| } |
There was a problem hiding this comment.
CommitAsync now owns doing this.
| RenameTrackingDismisser.DismissRenameTracking(Workspace, Workspace.GetOpenDocumentIds()); | ||
|
|
||
| // Log how long the full rename took. | ||
| _inlineRenameSessionDurationLogBlock.Dispose(); |
There was a problem hiding this comment.
this moved into EndRenameSessions_MustBeCalledOnUIThread
| @@ -867,84 +881,51 @@ private async Task CommitCoreAsync(IUIThreadOperationContext operationContext, b | |||
| } | |||
|
|
|||
| // The user hasn't canceled by now, so we're done waiting for them. Off to rename! | |||
| using var _ = operationContext.AddScope(allowCancellation: false, EditorFeaturesResources.Updating_files); | |||
| using var _1 = operationContext.AddScope(allowCancellation: true, EditorFeaturesResources.Updating_files); | |||
There was a problem hiding this comment.
changed this logic around slightly (to also allow more time for user to cancel if they want). Stages are:
- compute tree/text changes to make to the workspace. (still cancellable)
- jump to ui thread.
- make things non-cancellable.
- pre-cleanup (rollback to before linked edits were made)
- try apply to workspace.
- postcleanup.
| RenameLogMessage.UserActionOutcome outcome, | ||
| bool previewChanges, | ||
| Func<Task> finalCommitAction = null) | ||
| Action applyChangesOpt) |
There was a problem hiding this comment.
- the applyChanges callback was made synchronous.
- the function now required being on the UI therad.
these two things now make this entirely safe to call, without any sort of ugly .Wait from Cancel.
Inside CommitAsync, we do all the actual async work we need, on the BG, prior to calling into this once we've switched back to the UI thread,
| DismissUIAndRollbackEditsAndEndRenameSessionAsync( | ||
| RenameLogMessage.UserActionOutcome.Canceled, previewChanges: false).Wait(); | ||
| DismissUIAndRollbackEditsAndEndRenameSession_MustBeCalledOnUIThread( | ||
| RenameLogMessage.UserActionOutcome.Canceled, previewChanges: false); |
There was a problem hiding this comment.
no need for Wait anymore. This funciton is no longer async. It no longer attempts to get back to the UI thread (it requires it is already there from callers). And it has no async callback function to execute. So it's fully synchronous and simple.
| /// <summary> | ||
| /// Dismisses the UI, rolls back any edits, and ends the rename session. | ||
| /// </summary> | ||
| private void DismissUIAndRollbackEditsAndEndRenameSession_MustBeCalledOnUIThread( |
There was a problem hiding this comment.
How about call it DismissUIAndRollbackEditsAndEndRenameSessionOnUIThread?
Alternate approach to #74946.