Skip to content

Revert "Move workspace file removal to BG to avoid UI delay"#59472

Merged
jasonmalinowski merged 1 commit into
mainfrom
revert-59211-bgFileRemoval
Feb 11, 2022
Merged

Revert "Move workspace file removal to BG to avoid UI delay"#59472
jasonmalinowski merged 1 commit into
mainfrom
revert-59211-bgFileRemoval

Conversation

@jasonmalinowski
Copy link
Copy Markdown
Member

Reverts #59211.

This is causing regressions in DDRITs. When we are applying file edits as a part of code model or other features like code fixes, we in rapid succession open the file in an invisible editor, make changes in that editor, and then close it again. Because the open/close is now happening asynchronously, we end up with workspace changes from the background thread in places that don't expect it. I've seen a few odd behaviors under the debugger such as:

  1. Places in CodeModel that grab CurrentSolution more than once, assuming the documents are the same and thus have the same trees. Since the snapshot changes underneath them, we throw exceptions.
  2. Places where we are calling TryApplyChanges, but the intervening open/close means we are failing those (silently), resulting in later things throwing exceptions.

I think we can salvage the PR, but it's going to take some more work to ensure we don't break other things, and also to clean up some of the other issues discovered while debugging this. That won't happen quickly, so let's revert since we're breaking divisional tests here.

@jasonmalinowski jasonmalinowski requested a review from a team as a code owner February 11, 2022 01:09
@ghost ghost added the Area-IDE label Feb 11, 2022
@jasonmalinowski jasonmalinowski self-assigned this Feb 11, 2022
@jasonmalinowski jasonmalinowski merged commit 6ad4a16 into main Feb 11, 2022
@jasonmalinowski jasonmalinowski deleted the revert-59211-bgFileRemoval branch February 11, 2022 04:36
@ghost ghost added this to the Next milestone Feb 11, 2022
@RikkiGibson RikkiGibson modified the milestones: Next, 17.2.P2 Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants