Potential data loss for files that changed on disk meanwhile after a backup was made

[bpasero]

Following scenario:

• user types into a file and quits without saving
• the file changes on disk from another program without Code running
• user starts again and finds the file dirty as he was leaving it
• user saves and everything is fine

=> this basically bypasses our check to prevent dirty writes to files that have changed meanwhile
=> maybe the backup should store the etag of the file (mtime + size) as part of the backup in the meta data section of the backup (where we should also store the file path) so that this check is not bypassed.

[Tyriar]

Yes a check and a dialog would probably be ideal. Related: #14054

[bpasero]

@Tyriar you do not really need to build any UI as we already have UI in place if you try to save with the file on disk having changed. All we need is to record the mtime of the file from where the backup started and then restore that when we restore the backup. Currently we just take the mtime from the file on disk and then restore the backup, but that is obviously not right.

[chuckhacker]

What is the current behavior if the file changes while it is open (is there some sort of inotify listener?)? It's probably better to catch this as soon as possible, rather than waiting for the user to try to save it, so that the user doesn't make massive amounts of changes in a potential three-way merge scenario.

Maybe the requirements can be transferred over to the Backup service feature/story?

[berdon]

From the HN post on 1.8 update:

Sublime Text 3 checks to see if the currently opened version differs from the file system. If it does, it prompts the user to keep the current version or replace it with the disk version. VS Code just replaces the current version with the disk version and doesn't retain history so you can't undo this.

I lost about an hour of work yesterday because I had a file open in both VS Code and Sublime Text. Pressing save in Sublime Text wiped out all changes. :(

Also, please don't take my comparisons to Sublime Text 3 as a negative implication. It simply instilled an expected behavior that is easy to convey by making the comparison...

[bpasero]

Here is how file changes outside of VSCode have an impact (there are file watchers in place that track changes to any file in the opened folder + any opened file that is not within the folder):

• if the file is dirty in VS Code, we do not touch the users changes but when trying to save, Code will inform that the changes on disk are newer and will offer a diff-presentation of the changes on disk compared to the changes by the user. there are actions to accept or reject
• if the file is opened in VS Code and not dirty, we just update the contents without notification
• if the file is not opened in VS Code, we do not react to the file eventing

[berdon]

Hm, it's hard to say what happened because of the external interaction with Sublime Text.

[greggman]

another option, don't save modified files. Like 99% of apps, just pop up a dialog on quit "You have unsaved files". Could be simple "You have modified buffers, Exit Y? N?" or more complex, example

I prefer that solution, or at least the option (VSCode asking on exit and never saving unmodified buffers) over trying to make it's auto saving on quit work. Not saying that feature shouldn't be fixed, only saying that I'd prefer to turn off. Saving backups is good, restoring backups automatically that might be out of date, not so much.

[Tyriar]

Not saying that feature shouldn't be fixed, only saying that I'd prefer to turn off.

@greggman you should turn it off:

"files.hotExit": "off"

[DaveEM]

@Tyriar @bpasero Is there an update on when this issue is going to be fixed? This is a data loss issue. I just lost 30 minutes of work due to an unintentional clobber of work done outside of VS Code. Should hotExit be turned off until this is fixed?

[gjsjohnmurray]

I hope this will be addressed soon. The upcoming 1.34 release brings better dirty write handling in scenarios where FileSystemProvider extensions are sourcing the files, which I think will increase the likelihood of people getting bitten by this bug and ending up inadvertently stomping on each others work as a result of hot-exiting with unsaved changes and then resuming later.

[erturkk]

+1 on this is a data loss issue. I lost about a day's worth of work yesterday. VSC was my favorite editor. Now I will have to think twice about opening VSC again and just recommended VSC to friends this week and now have to warn them :)

What's holding us from implementing a fix? Looks like the bug is open for more than 2 years.

Thanks,

[bpasero]

@gjsjohnmurray I disagree that the recent changes in the file service make this bug appear anymore often than before. In fact, up until now, files saved through custom file providers (extensions) never had any protection of dirty writes, so I would argue we improved the behaviour here for custom file providers and not the other way around. That said, we always had hot-exit for files with custom scheme, nothing changed there.

@erturkk how did that happen exactly? I think in most cases at work, files are under some kind of source control system giving you the ability to see the changes before pushing them, no?

[gjsjohnmurray]

I disagree that the recent changes in the file service make this bug appear anymore often than before.

@bpasero My thinking is, once dirty write protection for custom file providers lands in 1.34 more people will be willing to use VSCode in scenarios where they need that protection. And then if they're configured to use hot-exit (which is the default), one day they may get bitten by this bug.

[erturkk]

@bpasero this bug hit me before I even checked in the file for the first time. Source control might help recover the file but that will be after the fact and depending on how recent the check-in was, one will lose data.

You should be able to replicate the problem easily. Let me define two actions:

• external changes: changes made to the file with an external editor.
• VSC changes: changes made to the file while it is open in VSC.

Open a file in VSC, edit it but don't save it and don't close the tab. Quit VSC. Use another editor to open the same file, modify it, save it, close this editor. Then open VSC again, and you will see the file is already opened but it does not reflect the changes made with the other editor. If you save the file now, you just overwrote the changes you made externally.

For me, I opened the file in VSC Thursday. I don't remember if I made any changes or not, but I quit VSC while the tab was open (this step is important). On Friday, I modified the same file with another editor whole day. Friday evening, I wanted to work on the same file and check in the first version. So I opened VSC, file was already opened so I started modifying from the top (i.e. add change comments, etc.). As I was editing I was also periodically saving (CTRL-S). Once I reached to the section of the code where I made changes on Friday, I realized the file was still the old file. Then it hit me changes I made on Friday were gone :)

Hope this helps, let me know if you need more info.

[greggman]

For me the steps were

1. Edit file in VSC, quit
2. git pull origin master # new version of files come down
3. run VSC. # vsc restores saved file silently replacing new version

If I now save in VSC and commit I just screwed my repo.

There is no other software on my enter system in the history of owning software for 35 years that has this behavior. I get that it's an interesting idea but it should arguably be off by default and like most software ask if you want to save changed files on exit. Again I know that's an option. I'd argue it should be the default since it's what people are used to coming from other software.

Otherwise if you want a fix then, at step 3 above, VSC should notice the file changed (maybe by date if not by content) and warn before loading the backed up file. As it IT SILENTLY DELTETS DATA

How many more people have to lose their work before this gets resolved?

[erturkk]

VSC could calculate a hash when file is opened for edited and save this if user quits without saving. Upon restarting, we compare and if different, then issue a warning to the user.

[bpasero]

I am fully aware how to reproduce this and do not get me wrong, I agree this is an issue, otherwise this issue would have been closed by now.

If you think it is too risky to have this behaviour, you can always turn off hot.exit in settings:

[bpasero]

#73123 is up to fix this.

[bpasero]

via #73123