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

Data loss: Custom editors do not properly deal with extension host restarts #122992

Closed
bpasero opened this issue May 5, 2021 · 12 comments
Closed
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug custom-editors Custom editor API (webview based editors) important Issue identified as high-priority verified Verification succeeded
Milestone

Comments

@bpasero
Copy link
Member

bpasero commented May 5, 2021

Steps to Reproduce:

  1. install an extension that leverages custom editors (non-text based, like Luna paint or Hex editor)
  2. open a folder
  3. open a file in hex editor or luna paint
  4. make the editor dirty
  5. add a folder to the workspace
  6. the extension host restarts
  7. click on another editor
  8. click back into the custom editor

=> the custom editor is gone and so is the data

image

@bpasero bpasero added bug Issue identified by VS Code Team member as probable bug custom-editors Custom editor API (webview based editors) labels May 5, 2021
@bpasero bpasero added the important Issue identified as high-priority label May 5, 2021
@bpasero bpasero changed the title Custom editors do not properly deal with extension host restarts Data loss: Custom editors do not properly deal with extension host restarts May 5, 2021
@bpasero
Copy link
Member Author

bpasero commented May 5, 2021

Unfortunately this can result in data loss as you can see from the video below:

recording

@lramos15
Copy link
Member

lramos15 commented May 5, 2021

I can't reproduce this using luna paint.

@bpasero
Copy link
Member Author

bpasero commented May 5, 2021

Here is how it reproduces for me with Luna paint running out of sources (auto save disabled):

recording (1)

@lramos15
Copy link
Member

lramos15 commented May 5, 2021

Hmm. Seems like a race condition, I follow those exact steps and still can't repro. I would be willing to investigate with you.

@bpasero
Copy link
Member Author

bpasero commented May 6, 2021

The more I think about it, the more I feel we really should stop restarting extension hosts: #69335

@sbatten
Copy link
Member

sbatten commented May 6, 2021

We must restart the extension host in the event the workspace is no longer trusted, so perhaps custom editors need to participate in that transition to avoid data loss

@bpasero
Copy link
Member Author

bpasero commented May 12, 2021

Fyi, yesterday I pushed support for IFileWorkingCopyManager#destroy which returns a Promise and is used for notebooks, e.g. when an extension is uninstalled or gets disabled. The gist is, this method will:

  • unregister any working copy it owns
  • trigger a save for dirty working copies
  • and if that fails, will resolve any existing backup for the working copy and save that to disk instead

However, I think this issue is more tricky for (non-text) custom editors, because we do not really know how the extension has implemented the save method to persist the changes to disk.

If we cannot restore the dirty editor properly after the extension host has restarted, we should at least make sure to reopen the editor with the dirty contents (= backup) after the extension host has started again.

@lramos15
Copy link
Member

From my discussion with Matt it appears the solution to this issue revolves around persisting the model managers upon extension host restart and syncing the models and webviews to the new ext host.

@Ark-kun
Copy link

Ark-kun commented May 24, 2022

Would the issue be solved if the custom editor was just resolved again (call resolveCustomTextEditor) for all documents opened in custom editors?

@bpasero
Copy link
Member Author

bpasero commented Apr 6, 2023

One idea from @alexdima was to introduce a participation model for extension host restarts: any component can veto the restart and thus custom editors could show a modal dialog to users in case this is needed and veto the restart.

@bpasero
Copy link
Member Author

bpasero commented May 25, 2023

#182558 addressed this 👍

@bpasero bpasero closed this as completed May 25, 2023
@Tyriar
Copy link
Member

Tyriar commented Jun 1, 2023

Verified, but I think #183778 is a core problem.

@Tyriar Tyriar added the verified Verification succeeded label Jun 1, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jul 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug custom-editors Custom editor API (webview based editors) important Issue identified as high-priority verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

6 participants