The default bug view has changed. See this FAQ.

AsyncTask onPostExecute sometimes runs on GeckoBackgroundThread instead of main thread

RESOLVED FIXED in Firefox 15

Status

()

Firefox for Android
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Margaret, Assigned: bnicholson)

Tracking

Trunk
Firefox 16
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox15 fixed, blocking-fennec1.0 -, fennec15+)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
I'm filing this as a follow-up to bug 746946. We landed a band-aid patch there, but a real solution involves finding out why this is happening, and perhaps using GeckoAsyncTask instead of AsyncTask to ensure onPostExecute always gets run on the main thread. However, to make this switch, we'd need to implement cancel() for GeckoAsyncTask.

Updated

5 years ago
blocking-fennec1.0: --- → ?
tracking-fennec: --- → 15+
blocking-fennec1.0: ? → -
(Assignee)

Comment 1

5 years ago
Created attachment 639931 [details] [diff] [review]
debug patch

I just ran into this after landing patches in bug 769145. Here's the STR I'm using - they only seem to work on my Galaxy Nexus:

1) Go to Manage Applications > Fennec, and click "Clear data"
2) Open Fennec
3) Quickly tap the AwesomeBar
4) Wait a few seconds
5) If there's no crash, click back, then go to step 3 (if the favicon on about:home has loaded, it's too late; start over from step 1)

This crashes fairly reliably for me with the signature from bug 746946. I followed these STR using the attached patch, and here's the output I see when it crashes:

D/BRN     ( 1056): 1 calling from main thread? true
D/BRN     ( 1056): 2 calling from main thread? false
D/BRN     ( 1056): 3 calling from main thread? false

This seems like a bug in AsyncTask since, according to [1], onPostExecute() should be called on same thread as the AsyncTask (which is the UI thread here). This led me to [2], which validates that it is, in fact, a recognized Android bug. I tried the solution given by romainguy, and it fixes the problem.

[1] http://developer.android.com/reference/android/os/AsyncTask.html
[2] http://code.google.com/p/android/issues/detail?id=20915
Assignee: nobody → bnicholson
(Assignee)

Comment 2

5 years ago
Created attachment 639933 [details] [diff] [review]
patch

Here's the suggested fix. Hopefully this fixes other similar bugs.
Attachment #639933 - Flags: review?(blassey.bugs)
(Assignee)

Comment 3

5 years ago
(In reply to Brian Nicholson (:bnicholson) from comment #1)
> I just ran into this after landing patches in bug 769145. Here's the STR I'm
> using - they only seem to work on my Galaxy Nexus:

s/Galaxy Nexus/Galaxy S/

This is the same phone, and very similar steps, to what Aaron used to reproduce bug 769145.
Comment on attachment 639933 [details] [diff] [review]
patch

Shouldn't this be in GeckoApplication instead of GeckoActivity? See:
http://code.google.com/p/android/issues/detail?id=20915#c7
Comment on attachment 639933 [details] [diff] [review]
patch

As Finkle said in comment 4, this should be done in Application::onCreate(). I'd also note that this doesn't guaranty to work around the problem, just makes it much less likely that we'll hit it. The sure fire way to fix this is to stop using AsyncTasks and use GeckoAsyncTasks.
Attachment #639933 - Flags: review?(blassey.bugs) → review-
(In reply to Brad Lassey [:blassey] from comment #5)
> The sure fire way to fix this is to stop using AsyncTasks and use GeckoAsyncTasks.

I think we needed a few more features in GeckoAsyncTask to be able to use it in more places.
(Assignee)

Comment 7

5 years ago
Created attachment 639935 [details] [diff] [review]
patch v2

Moved to GeckoApplication
Attachment #639933 - Attachment is obsolete: true
Attachment #639935 - Flags: review?(blassey.bugs)
Attachment #639935 - Flags: review?(blassey.bugs) → review+
(Assignee)

Comment 8

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/da718420b647
(Assignee)

Comment 9

5 years ago
Comment on attachment 639935 [details] [diff] [review]
patch v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): none
User impact if declined: potential to crash using AsyncTask
Testing completed (on m-c, etc.): just landed on inbound
Risk to taking this patch (and alternatives if risky): very low risk
String or UUID changes made by this patch: none
Attachment #639935 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/da718420b647
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 16
Comment on attachment 639935 [details] [diff] [review]
patch v2

[Triage Comment]
Given the low risk profile and the mobile team's tracking-fennec designation, let's uplift to Aurora 15.
Attachment #639935 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 12

5 years ago
http://hg.mozilla.org/releases/mozilla-aurora/rev/6eb6a52e6f77
status-firefox15: --- → fixed
You need to log in before you can comment on or make changes to this bug.