Closed
Bug 1288186
Opened 8 years ago
Closed 8 years ago
Possible deadlock when restoring session
Categories
(Firefox for Android Graveyard :: Awesomescreen, defect, P1)
Tracking
(firefox47 unaffected, firefox48 unaffected, firefox49 fixed, firefox50 fixed)
RESOLVED
FIXED
Firefox 50
Tracking | Status | |
---|---|---|
firefox47 | --- | unaffected |
firefox48 | --- | unaffected |
firefox49 | --- | fixed |
firefox50 | --- | fixed |
People
(Reporter: jchen, Assigned: JanH)
References
Details
Attachments
(1 file)
RecentTabsAdapter can be created earlier than we call GeckoProfile.updateSessionFile (through AppCompatActivity.onCreate), and we'll deadlock as a result.
"main" prio=5 tid=1 Waiting
| group="main" sCount=1 dsCount=0 obj=0x64668ad0 self=0x41ae1928
| sysTid=6766 nice=0 cgrp=apps sched=0/0 handle=0x4010d154
| state=S schedstat=( 0 0 0 ) utm=63 stm=5 core=0 HZ=100
| stack=0xbe66e000-0xbe672000 stackSize=8MB
at java.lang.Object.wait(Native method)
- waiting on <0x64fcbc98> (a org.mozilla.gecko.GeckoProfile)
at org.mozilla.gecko.GeckoProfile.waitForOldSessionDataProcessing(GeckoProfile.java:624)
- locked <0x64fcbc98> (a org.mozilla.gecko.GeckoProfile)
at org.mozilla.gecko.home.RecentTabsAdapter.<init>(RecentTabsAdapter.java:78)
at org.mozilla.gecko.home.CombinedHistoryPanel.onCreate(CombinedHistoryPanel.java:121)
at android.support.v4.app.Fragment.performCreate(unavailable:-1)
at android.support.v4.app.FragmentManagerImpl.moveToState(unavailable:-1)
at android.support.v4.app.FragmentManagerImpl.moveToState(unavailable:-1)
at android.support.v4.app.FragmentManagerImpl.moveToState(unavailable:-1)
at android.support.v4.app.FragmentManagerImpl.dispatchCreate(unavailable:-1)
at android.support.v4.app.FragmentActivity.onCreate(unavailable:-1)
at android.support.v7.app.AppCompatActivity.onCreate(unavailable:-1)
at org.mozilla.gecko.GeckoActivity.onCreate(GeckoActivity.java:45)
at org.mozilla.gecko.GeckoApp.onCreate(GeckoApp.java:1331)
at org.mozilla.gecko.BrowserApp.onCreate(BrowserApp.java:564)
at android.app.Activity.performCreate(Activity.java:5231)
at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1087)
at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2148)
at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2233)
at android.app.ActivityThread.access$800(ActivityThread.java:135)
at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1196)
at android.os.Handler.dispatchMessage(Handler.java:102)
at android.os.Looper.loop(Looper.java:136)
at android.app.ActivityThread.main(ActivityThread.java:5001)
at java.lang.reflect.Method.invoke(Native method)
at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:785)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:601)
Updated•8 years ago
|
Priority: -- → P1
Assignee | ||
Comment 1•8 years ago
|
||
Ugh, yes. Am I right in assuming that postToBackgroundThread is still referring to *the* background thread (as opposed to *a* thread pool-like background thread)? In which case if my understanding is right we'd have to do the RecentTabsAdapter's wait from a completely separate thread (as opposed to just moving it into the following background thread runnable), because the call which finally relases that wait (https://dxr.mozilla.org/mozilla-central/rev/4c05938a64a7fde3ac2d7f4493aee1c5f2ad8a0a/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java#1398) is already running on *the* background thread, so if the RecentTabsAdapter is created first, we'd just lock up the background thread instead.
Also I'm wondering whether this might in fact be the reason, behind bug 1282292?
status-firefox47:
--- → unaffected
status-firefox48:
--- → unaffected
status-firefox49:
--- → affected
status-firefox50:
--- → affected
Component: General → Awesomescreen
See Also: → 1282292
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → jh+bugzilla
Assignee | ||
Comment 2•8 years ago
|
||
When reading the previous session file, the RecentTabsAdapter 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 RecentTabsAdapter happens to be initialised *before* GeckoApp.
Review commit: https://reviewboard.mozilla.org/r/66548/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/66548/
Attachment #8774219 -
Flags: review?(nchen)
Reporter | ||
Comment 3•8 years ago
|
||
Comment on attachment 8774219 [details]
Bug 1288186 - Do the RecentTabsAdapter wait from a separate thread.
https://reviewboard.mozilla.org/r/66548/#review63654
LGTM
::: mobile/android/base/java/org/mozilla/gecko/home/RecentTabsAdapter.java:126
(Diff revision 1)
> }
> });
> }
>
> private void readPreviousSessionData() {
> + final Thread parseThread = new Thread(new Runnable() {
Give the thread a name, either via the constructor or by calling setName.
Attachment #8774219 -
Flags: review?(nchen) → review+
Assignee | ||
Comment 4•8 years ago
|
||
Comment on attachment 8774219 [details]
Bug 1288186 - Do the RecentTabsAdapter wait from a separate thread.
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/66548/diff/1-2/
Assignee | ||
Comment 5•8 years ago
|
||
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/6f1eb74d5648
Do the RecentTabsAdapter wait from a separate thread. r=jchen
Keywords: checkin-needed
Comment 7•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
Assignee | ||
Comment 8•8 years ago
|
||
Hopefully fixed by the Aurora patch from bug 1288816.
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•