Closed Bug 1376442 Opened 7 years ago Closed 7 years ago

Session restore fails

Categories

(Firefox :: Session Restore, defect)

56 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
firefox-esr52 --- unaffected
firefox54 --- unaffected
firefox55 --- unaffected
firefox56 - fixed

People

(Reporter: Tobias.Marty, Unassigned)

References

Details

(Keywords: regression)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0
Build ID: 20170627030209

Steps to reproduce:

-Create new profile (or use an existing one)
-Sign in to Sync
-Open a few sites, open new tab page
-Confirm that session restore is enabled
-Close and restart Nightly


Actual results:

-First entry in hamburger menu shows "sign in to Sync" while according to options Sync is active. 
-Session is empty and one tab with the Home Page gets loaded. 
-New tab page is broken and only shows a search field, but no top sites.


Expected results:

Hamburger menu states active Sync state and the session gets restored.
Component: Untriaged → Session Restore
So I don't know if that makes sense, but after a lot of testing I've come to the conclusion that it happens once you remove the search bar from the interface. 

So new steps to reproduce: 
Use customize to remove the search bar from the interface and restart -> Multiple things are broken in the browser, as described.
WTF, weird bug. Anyway, can confirm:
Install today's Nightly -> remove search bar -> restart -> Nightly is toast.
Other things broken after above action:
Web Developer menu (and tools, not even keyboard shortcuts work)
Customize
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Resolution: --- → DUPLICATE
I'm not sure this is a dupe of bug 1376466 - even with the new Nightly that contains the backout there (and as such devtools etc are now working), my session restore still doesn't work.

Ryan, you mentioned earlier that the session restore was a separate issue? Is there another bug filed for it?
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: DUPLICATE → ---
[Tracking Requested - why for this release]: session restore issues

The regressing bug was bug 934967 I believe. I don't see any bugs hanging off it at this point in time for this issue, so let's just use this one.
Blocks: 934967
Flags: needinfo?(nnn_bikiu0707)
Keywords: regression
Hmm so switching back to the June 26th Nightly resulted in my previous session being restored.

I then tried purging the new jsonlz4 files (both in the root of the profile and sessionstore-backups) and running the June 27th Nightly respin (which included the fix for bug 1376466) then succeeded - the sessionstore was migrated to the jsonlz4 format.

So I guess the problem was that enough of the migration had occurred so that it appeared the migration had already been performed, but not quite enough to actually migrate the full session. Can the migration perhaps be made more resilient? (eg not create the new files unless the migration was actually successful?)
Hi :emorley, I'm the author of the patch for bug 934967. Is the steps to reproduce this bug is the same as comment 1 and comment 2 or is it different? If it is different, can you describe how to reproduce it?
Flags: needinfo?(nnn_bikiu0707) → needinfo?(emorley)
STR:
1) Have an existing session created in a Nightly build prior to bug 934967 landing (eg June 26th or earlier Nightly, or any beta/release version)
2) Open the June 27th Nightly prior to the respin (ie affected by bug 1376466).
3) Open the June 27th Nightly after the respin (or any later version, eg tomorrow's Nightly)

Expected:
Nightly failing to start correctly for any reason (eg bug 1376466) at the time of the .js -> .jsonlz4 migration (step 2) should not stop the session being restored correctly at step 3.

Actual:
At step 2, broken/empty .jsonlz4 session files are created, which means that at step 3 Nightly doesn't attempt to run the migration again, thereby causing data loss unless the user realises what has occurred and manually deletes the .jsonlz4 files to force another attempt at the migration.

--

Whilst hopefully bug 1376466 is a rare event, and not something that might affect more significant channels (eg beta/release), there presumable are other reasons the browser mail fail to start correctly, which could trigger the same issue.
Flags: needinfo?(emorley)
(In reply to Ed Morley [:emorley] from comment #10)
> STR:
> 1) Have an existing session created in a Nightly build prior to bug 934967
> landing (eg June 26th or earlier Nightly, or any beta/release version)
> 2) Open the June 27th Nightly prior to the respin (ie affected by bug
> 1376466).
> 3) Open the June 27th Nightly after the respin (or any later version, eg
> tomorrow's Nightly)
> 
> Expected:
> Nightly failing to start correctly for any reason (eg bug 1376466) at the
> time of the .js -> .jsonlz4 migration (step 2) should not stop the session
> being restored correctly at step 3.
> 
> Actual:
> At step 2, broken/empty .jsonlz4 session files are created, which means that
> at step 3 Nightly doesn't attempt to run the migration again, thereby
> causing data loss unless the user realises what has occurred and manually
> deletes the .jsonlz4 files to force another attempt at the migration.
> 
> --
> 
> Whilst hopefully bug 1376466 is a rare event, and not something that might
> affect more significant channels (eg beta/release), there presumable are
> other reasons the browser mail fail to start correctly, which could trigger
> the same issue.

Ok, so if I understand correctly, the migration from .js to .jsonlz4 is ok. But if something wrong happens after the session file is loaded and restored, users will be presented with an empty session. And that empty session is saved (or migrated to .jsonlz4).

So what we should do is:
1. If something wrong happens during the restore process (*)
 1.1 Don't save/migrate that session (i.e don't save to $Path.clean)
2. Else
 2.1 Save/migrate that session.
3. Move/migrate $Path.clean to $Path.cleanBackup
4. ...

Mike, is there a way to know (*)?
Flags: needinfo?(mdeboer)
(In reply to Bao Quan [:beekill] from comment #11)
> Mike, is there a way to know (*)?

I'm afraid not. The only way - which worked for :past who experienced something similar yesterday - is to
1. Shutdown Firefox.
2. Move all the .jsonlz4 sessionstore-related files out of the profile directory and leave the old .js files in there.
3. Then restart Firefox and it'll use the .js files and create the .jsonlz4 ones again using a correctly restored session.

This is indeed a manual process, but we really, genuinely, absolutely can't catch every possible error during startup (both immediate and deferred) when someone lands an erroneous patch, which happens to merge into Nightly. This spuriously happens, as demonstrated recently, but we can't fence that.
Flags: needinfo?(mdeboer)
Is there anything further to do here? 
Do we have any idea how many nightly users might be affected?
Flags: needinfo?(mdeboer)
There's nothing further to do here; it's essentially not the fault of sessionstore at all - just a side-effect that makes using Nightly exciting.

I've stated the mitigation plan above in comment 12 and it's been proven to work. If people are still upset to an uncomfortable degree, I'd recommend for them to move to Firefox Beta.
Flags: needinfo?(mdeboer)
(In reply to Mike de Boer [:mikedeboer] from comment #14)
> If people are still upset to an uncomfortable degree, I'd recommend
> for them to move to Firefox Beta.

Whilst there might not be much that could be done to avoid this particular scenario (and I appreciate you having taken a look), I feel like a blanket "well just don't use Nightly" statement is in direct contradiction with the current efforts to getting *more* people using Nightly. Yes Nightly will have more issues, but ideally very few that involve data-loss/broken-without-intervention profiles.
OK, thanks, I don't think relman needs to track this for the moment if it was a blip in Nightly.
(In reply to Bao Quan [:beekill] from comment #11)
> So what we should do is:
> 1. If something wrong happens during the restore process (*)
>  1.1 Don't save/migrate that session (i.e don't save to $Path.clean)
> 2. Else
>  2.1 Save/migrate that session.
> 3. Move/migrate $Path.clean to $Path.cleanBackup
> 4. ...
> 
> Mike, is there a way to know (*)?

(In reply to Mike de Boer [:mikedeboer] from comment #12)
> I'm afraid not.

Why not? Sessionstore knows when a session should be restored and should be able to detect when something went wrong, at least the most basic case of nothing getting restored at all, as happened here.
Flags: needinfo?(mdeboer)
(In reply to Mike de Boer [:mikedeboer] from comment #14)
> I've stated the mitigation plan above in comment 12 and it's been proven to
> work. 

Not for me. No session is saved at all, it doesn't matter how many times I delete jsonlz4 files.
FWIW, my experience was that deleting the jsonlz4 files restored my tabs, but session restore just failed again upon the next restart requiring the 'delete files' step repeatedly.  Setting browser.search.widget.inNavBar = true made it stop failing. (I removed the widget a few days of Nightly updates later; no further issues.)
(In reply to Dão Gottwald [::dao] from comment #17)
> Why not? Sessionstore knows when a session should be restored and should be
> able to detect when something went wrong, at least the most basic case of
> nothing getting restored at all, as happened here.

Yes, true, but I was talking about things that are immediately actionable. We don't have the resources available at the moment to work on improving this particular case.
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Flags: needinfo?(mdeboer)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.