792702 - java.lang.IllegalThreadStateException: Expected thread 1 ("main"), but running on thread <n> ("GeckoBackgroundThread) at org.mozilla.gecko.GeckoApp.assertOnThread(GeckoApp.java)

Status: VERIFIED FIXED

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

Description

[Scoobidiver (away)]

There's one crash in 18.0a1/20120919: bp-249094c2-7759-4ef3-817d-5f49c2120920.

java.lang.IllegalThreadStateException: Expected thread 1 ("main"), but running on thread 9 ("GeckoBackgroundThread)
	at org.mozilla.gecko.GeckoApp.assertOnThread(GeckoApp.java:2658)
	at org.mozilla.gecko.GeckoApp.assertOnUiThread(GeckoApp.java:2645)
	at org.mozilla.gecko.PromptService.show(PromptService.java:186)
	at org.mozilla.gecko.ActivityHandlerHelper$FilePickerPromptRunnable.run(ActivityHandlerHelper.java:240)
	at android.os.Handler.handleCallback(Handler.java:587)
	at android.os.Handler.dispatchMessage(Handler.java:92)
	at android.os.Looper.loop(Looper.java:130)
	at org.mozilla.gecko.util.GeckoBackgroundThread.run(GeckoBackgroundThread.java:31)

More reports at:
https://crash-stats.mozilla.com/report/list?signature=java.lang.IllegalThreadStateException%3A+Expected+thread+1+%28%22main%22%29%2C+but+running+on+thread+9+%28%22GeckoBackgroundThread%29+at+org.mozilla.gecko.GeckoApp.assertOnThread%28GeckoApp.java%29

Comment 1

[Chris Peterson [:cpeterson]]

kats, I think the FilePickerPromptRunnable should be posted to the UI thread' GeckoAppShell.getMainHandler(), not the GeckoBackgroundThread's GeckoAppShell.getHandler():

https://hg.mozilla.org/mozilla-central/annotate/6a2d1a3556b9/mobile/android/base/ActivityHandlerHelper.java#l151

I think we should rename these GeckoAppShell Handler getters to something clearer like getUiThreadHandler() and getBackgroundThreadHandler() (and remove getGeckoHandler() because it is not used).

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Sounds reasonable to me. Do you want me to do it or will you?

Comment 3

[Chris Peterson [:cpeterson]]

I can fix the crash, since I added the assertOnUiThread() call when fixing a related PromptService bug. :)

Aurora 17 and Beta 16 don't call assertOnUiThread() here, but they still have the bug. This bug is likely contributing to the CalledFromWrongThreadException topcrashes.

Comment 4

[Chris Peterson [:cpeterson]]

STR:
1. Load http://www.w3schools.com/tags/tryit.asp?filename=tryhtml_input_type_file
2. Press "Browse..." button
3. Choose a file
4. CRASH!

Comment 5

[Chris Peterson [:cpeterson]]

Attachment: post-FilePicker-to-UI-thread.patch

Post FilePicker dialog to UI thread, not GeckoBackgroundThread.

Comment 6

[Chris Peterson [:cpeterson]]

btw, my fix reveals a bunch of FilePicker StrictMode warnings. I confirmed that these warnings at not a regression from my patch, so I filed new bug 792973.

Comment 7

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dbed92805ae7

Comment 8

[Chris Peterson [:cpeterson]]

Comment on attachment 663128 [details] [diff] [review]
post-FilePicker-to-UI-thread.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 786312
User impact if declined: Nightly 18 has 100% repro crashes when trying to use FilePicker (e.g. Bugzilla attachment). Aurora 17 and Beta 16 don't immediately crash here, but they still have the logic bug. This bug is likely contributing to 17 and 16's CalledFromWrongThreadException topcrashes.
Testing completed (on m-c, etc.): Tested locally with 100% STR
Risk to taking this patch (and alternatives if risky): Low risk. Android-only java one-liner. Some UI code was running on a background thread, but it should only run on the UI thread.
String or UUID changes made by this patch: N/A

Comment 9

[Lukas Blakk [:lsblakk] use ?needinfo]

Triage drive-by:  Likely we can take this on Monday if it has been landed on trunk before then without any known fallout.

Comment 10

[Chris Peterson [:cpeterson]]

(In reply to Lukas Blakk [:lsblakk] from comment #9)
> Triage drive-by:  Likely we can take this on Monday if it has been landed on
> trunk before then without any known fallout.

I am testing a band-aid fix now.

Comment 11

[Chris Peterson [:cpeterson]]

^ oops! My comment about a "band-aid fix" was intended for bug 747629, not this bug! <:)

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/dbed92805ae7

Comment 13

[Chris Peterson [:cpeterson]]

Comment on attachment 663128 [details] [diff] [review]
post-FilePicker-to-UI-thread.patch

(In reply to Lukas Blakk [:lsblakk] from comment #9)
> Triage drive-by:  Likely we can take this on Monday if it has been landed on
> trunk before then without any known fallout.

lsblakk, renomming for aurora? and beta?

I landed my patch on 09-21, so the first build to include my fix was 09-22. Before my fix, I see about a dozen crash reports in builds 09-20 and 09-21. After my fix, I see no crash reports in builds 09-22, 09-23, or 09-24.

Comment 14

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/628cc50702f8
https://hg.mozilla.org/releases/mozilla-beta/rev/c085464581b6

Comment 15

[Cristian Nicolae (:xti)]

This crash doesn't occur on any branch if I perform the STR from comment #4. Closing bug as verified fixed on:

Firefox 20.0a1 (2012-11-21)
Device: Galaxy S2
OS: Android 4.0.3