-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Mitigate corruption of scores on save #24908
Comments
Surely we should just fix the underlying problem, I don't know of any other application, Windows or otherwise, that somehow ends up writing all 0s to a file when it's meant to actually store useful data. |
Well... The issue goes unnoticed until the user opens a corrupt score file. So, no, we don't have any steps for replication, at least not that I am aware of. With the proposed changes, we might shed some light if we get the users to cooperate. There are too many unknowns. It will be very interesting first to know whether the save will suceed or not if the user retries it immediately. We also need to know if the corruption happens on the temp file or after the temp file is moved over the original file. And receive some uncompressed folders at the moment of the failed save. |
I've added a Draft pull request for people to look at what I have so far and help me refine it. So far I've only covered the save the entire score locally scenario. |
No; that's the problem. This issue has existed for years now, even in MS3 times, and thus probably also before. See also #16339 (which also states that there are more variants of this issue than the "only zeros" variant). I've looked at |
FWIW, I actually don’t remember there were any reports of this prior to MU3. And not even immediately upon release of 3.0 - maybe not for another year or so. I could be wrong, maybe @Jojo-Schmitz has more insight. But it’s been frustrating as indeed no one has any clue how to reproduce this. |
When was the mscz format first introduced? I assume it definitely only happens with that, and not mscx files... |
Before my time. And yes, I've never heard of this reported with MSCX< although that's surprising because only a handful of people in the world use that with any regularity. But again, we had no problem with MSCZ for years, then suddenly started seeing it pretty regularly - a report every few weeks or so. A possible clue is that while it's exceedingly rare ovewall, there are people who say it has happened multiple times. That doesn't seem likely to be random chance. So I'm guessing it's some interaction involving a system dependency. Like some backup service being run, or a particular device driver, etc. |
See So since 0.9.3, September 2008 |
Seems indeed to have started with 3.0, even with the Alpha, see https://musescore.org/en/node/271185 Nah, much longer, see https://musescore.org/en/node/38096, November 2014 See also #16339 |
I assume that Musescore does not write null bytes to disk due to a software bug in Musescore.
So I have doubts that checking the file after writing will help avoid the issue or improve the situation. |
Thank you all for the discussion and the links. And for your review comments, I'll address those shortly. Wow, I didn't know there were corruptions other than the all zeros scenario and that this issue is much older than I thought! @mercuree Checking the file won't avoid the issue but will significantly improve the situation. First of all, the user will be alerted so the issue won't be going unnoticed any more. Then, they will be able to retry the save which might succeed: if it does, great for the user, great for us as we will then know it. If the retry doesn't fix it, and the corruption happens before the original file is overwritten (which I very much hope and believe), the users will still have their last successful save intact. If the retry doesn't fix the issue and the corruption happens after the move, at least we will alert the user (and know it). At this point they can try saving to a different location, save as an uncompressed dir to try to save their score, contact us and send us something, etc. If the retry doesn't fix it, it might be a repeatable and reproducible problem with the users' scores at that moment, meaning if they find a way to send us what they are trying to save, we might be able to reprodice it and catch it. If retrying the save doesn't fix the issue, a user could also undo their last change in MSS and try saving again, then undo further and see if at some point the file will save (after ensuring they have a backup of their original file). What are the issues with saving to an uncompressed folder? |
Do we want to verify anything besides the main score file without parsing it (which is what I have so far)? Like enumerate all the files in the zip and read them? |
I looked at the Notepad++ fix for what ostensibly may be the same issue (though it seems likely there's more than one root cause by the many bug reports received). Basically they add an extra flag "c" to the fopen mode parameter, which seems to be a Windows-specific thing to change the commit behaviour on calling fflush, and then an explicit call to fflush before fclose. MuseScore doesn't use fopen/fflush though, rather QFile (part of Qt), which internally doesn't even use fopen/fflush for Windows, and even explicitly has a comment (in qfsfileengine_win.cpp, nativeFlush) about not needing to flush under Windows. QFile does seem to support working in "stdlib" mode, whereby the application calls fopen and passes the file handle in though, not sure if it's worth trying out. The other thing I'd note that may be relevant is that when running under the debugger I've noticed that MuseScore quite often "crashes" on shutdown in a background thread - but the default Windows behaviour is such that if you don't have a debugger attached it's not even obvious to a user that such a crash has occurred. I don't know if MacOS or Linux is similar in this regard. It seems possible that if the main thread is trying to save your score but a crash happens in a background thread you're likely to end up with a corrupted file...but in that case it would be while writing to .mscz_saving, and presumably the attempt to rename it to .mscz would never happen. So it's doubtful that's a root cause. It does seem that having some sort of sanity check before renaming the .mscz_saving file to .mscz (in NotationProject::doSave) would be worth considering though. |
…e and only if unsuccessful to not degrade save times + code review changes
…e and only if unsuccessful to not degrade save times + code review changes
…e and only if unsuccessful to not degrade save times + code review changes
… out in the prescribed order (not on Windows)
…iginal file and not the temporary "_saving" file
…to keep things simple
…Save #24908: Validate saved files and alert user
…e and only if unsuccessful to not degrade save times + code review changes
… out in the prescribed order (not on Windows)
…iginal file and not the temporary "_saving" file
…to keep things simple
Hi all! I've just realized something. When we show the dialog about the corrupted save, MSS has already corrupted the original score but there should be a healthy backup file in the backup folder. Now, if the user decides to retry the save by clicking "Retry" on the dialog, which is the expected behavior, then if the retried save fails too, this time the healthy backup file will be overwritten with the now corrupted original score. I am wondering if we want to reopen this issue and quickly make a change such that the retried saves DO NOT create a backup file (unless there are other ideas)? |
Issue type
Opening/saving issue
Description with steps to reproduce
As you know, there have been occasional but not rare reports of corrupted files after saving where the saved files are full of zeros and therefore unusable. Many users have lost work due to this problem with either backups not being available or corrupted as well. Let's try to mitigate this and find out what is causing it.
What we have so far:
I am proposing the following changes:
The validation check could vary. I am thinking that just reading the main score file from the zipped file without parsing it should be enough. (Again, we are dealing with only one scenario where the entire file is full of 0-s).
For the testing, I propose that we have MSS deliberately write a file full of zeros if the file name ends with "- CORRUPTED.mscz" for example (at least this is what I have already implemented). For the release, we will remove this code.
For the dialog to show to the user, I am thinking of a series of dialogs but as @cbjeukendrup mentioned, this might be too much. My reasoning for a series of small dialogs (like a wizard to walk the user through the corruption situation) is this:
This seems a lot of information to me that will hardly fit in one dialog and even if it does fit, it might be overwhelming to the user.
P.S. I have implemented most of the propsed changes already except for the dialogs (a simple dialog for now).
Supporting files, videos and screenshots
n/a
What is the latest version of MuseScore Studio where this issue is present?
4.4.2
Regression
No.
Operating system
Windows
Additional context
No response
Checklist
The text was updated successfully, but these errors were encountered: