Fulguris
Fulguris copied to clipboard
Slion
Auto-backup session files before writing
might fix #193
This changes behavior when writing bundle to file.
Previously the old file was overwritten, which sometimes resulted in broken files and all tabs of the current session being lost.
Now the old file is renamed with .backup suffix before the new one is written. When reading the session file results in an error, the backup file is used instead.
I did not test this, other than checking whether the backup file is created correctly.
I had in mind to attempt a very similar fix, thanks for taking the initiative. We should be able to test it though one way or another. I have not looked at the code yet. We could just display a Toast when the backup session is loaded if only as a means for us to test it. I'm pretty sure I can reproduce that bug fairly easily. It's just matter of finding the time to try it out.
Hmm, if you can reproduce the bug easily, could you try it and take a log to see where it crashes? (I did this PR after losing a session, but didn't think of checking error logs)
Because right now I'm thinking that a corrupt file might not crash here, but later, e.g. in TabsManager.loadSession. So the retry with backup files should rather/additionally be done somewhere else.
Steps to reproduce are all in #193. There are actually two ways I could reliably reproduce it. Sorry can't invest time into that right now.
So how can I make the system restart the app for me? I don't find any System navigation settings on my Lineage 16.0 / Android 9 phone.
That's one way to reproduce it. The other one is to just have a large session, 150 tabs worked for me. Then from the session go to the task manager and close Fulguris as fast as possible. Turns out the user task manager on Android just kills apps so that should prevent the session from being saved. See: https://github.com/Slion/Fulguris/issues/193#issuecomment-1101050478
I am at more than 200 tabs now, and either nothing is written, or the entire file without any corruptions.
Yes that's what I would expect. When nothing is written then you lose all your tabs.
Well, with nothing is written I meant that the sessions file is just not updated because the app is killed too early. So I only lose changes to tabs since the last save, but the file is not corrupted.
Still didn't manage to reproduce it...
Anyway, when going through it again I noticed this is only dealing with file reading errors, so it won't deal with garbage data that may come from the app being killed while writing. So more changes are needed, I'll change this into a draft.
Changed:
- don't delete the backup file when calling
saveState - retry from backup file on errors in
loadSessionandloadSessions
Now it might actually help.
What's the status of this? Do we want to merge it? I intend to rewrite the tab persistence engine but it could be while until I get around it. I lost 500 tabs yesterday 😁
I'll need to go throught the changes again... I use a modified version of this, and want to first compare what I actually did there (was a while ago, and looks like part of this was actually logging).
Anyway, one thing I can remember was that I found in some cases TabsManager.onStop() is called before initializeTabs. This will store some bad sessions file, but probably doesn't destroy any session file, because the session file name is different. And I think the broken sessions file is already caught in recoverSessions.
My assumption with #193 had been that the process is interrupted mid-save and you have a corrupt session file. So I took care of not saving directly to the session file but saving to another file then renaming it. To my surprise it did not help. Turns out there is a weird life cycle sequence that could trigger a shutdown before a saveState. So we were shooting ourselves in the foot. Deleting all tabs before saving the session.
I'll close that PR and commit a couple of fixes. We could always reopen it if need be.