Closed Bug 1288816 Opened 8 years ago Closed 8 years ago

Possible deadlock in former RecentTabsPanel when restoring session

Categories

(Firefox for Android Graveyard :: Awesomescreen, defect, P1)

All
Android
defect

Tracking

(firefox47 unaffected, firefox48 unaffected, firefox49 fixed, firefox50 unaffected)

RESOLVED FIXED
Firefox 49
Tracking Status
firefox47 --- unaffected
firefox48 --- unaffected
firefox49 --- fixed
firefox50 --- unaffected

People

(Reporter: JanH, Assigned: JanH)

References

Details

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #1288186 +++ Firefox 49 doesn't have the RecentTabsAdapter, but the RecentTabsPanel might very well have the same deadlock potential (https://dxr.mozilla.org/mozilla-aurora/rev/c1cee3d0990479215fe757140aaaad3916c362fc/mobile/android/base/java/org/mozilla/gecko/home/RecentTabsPanel.java#333).
When reading the previous session file, the RecentTabsPanel waits for GeckoApp (initialisation runs on the main thread) to have actually moved the previous session file to its correct location (happens from the background thread). Hence we can't do the wait from either of those two threads, because we risk a deadlock otherwise if the RecentTabsPanel happens to be initialised *before* GeckoApp. Review commit: https://reviewboard.mozilla.org/r/66672/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/66672/
Attachment #8774221 - Flags: review?(nchen)
https://reviewboard.mozilla.org/r/66672/#review63666 ::: mobile/android/base/java/org/mozilla/gecko/home/RecentTabsPanel.java:261 (Diff revision 1) > mClosedTabs = closedTabs; > > // The fragment might have been detached before this code > // runs in the UI thread. > if (getActivity() != null) { > + Thread updateCursorThread = new Thread(new Runnable() { Can you move this into a separate method so the same code can be used here and also in load() above?
Comment on attachment 8774221 [details] Bug 1288816 - Aurora patch - Do the RecentTabsPanel wait from a separate thread. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/66672/diff/1-2/
Comment on attachment 8774221 [details] Bug 1288816 - Aurora patch - Do the RecentTabsPanel wait from a separate thread. https://reviewboard.mozilla.org/r/66672/#review63822
Attachment #8774221 - Flags: review?(nchen) → review+
Comment on attachment 8774221 [details] Bug 1288816 - Aurora patch - Do the RecentTabsPanel wait from a separate thread. Approval Request Comment [Feature/regressing bug #]: bug 905223, mobile session restoring [User impact if declined]: Firefox might deadlock and hang during Startup. [Describe test coverage new/current, TreeHerder]: manual [Risks and why]: Low, this just moves a wait for a file operation into a dedicated thread, so we don't block the main or (global) background thread. [String/UUID change made/needed]: none
Attachment #8774221 - Flags: approval-mozilla-aurora?
Comment on attachment 8774221 [details] Bug 1288816 - Aurora patch - Do the RecentTabsPanel wait from a separate thread. This patch fixes a potential deadlock and hang issue. Take it in aurora.
Attachment #8774221 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: