Closed Bug 719795 Opened 8 years ago Closed 8 years ago

SMS related Android crashes. Device rotation in XUL builds and language change in native builds [@ TouchBadMemory][@ GeckoSmsManager.init]

Categories

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

Firefox 12
ARM
Android
defect
Not set
critical

Tracking

(firefox11 unaffected, firefox12 fixed)

VERIFIED FIXED
Firefox 12
Tracking Status
firefox11 --- unaffected
firefox12 --- fixed

People

(Reporter: wgianopoulos, Assigned: mbrubeck)

References

()

Details

(4 keywords, Whiteboard: [mobile-crash][native-crash][not-fennec-11])

Crash Data

Attachments

(2 files)

bp-8ed31cc2-df21-43a5-a3b7-f70742120120

This crash is easily reproducable on my Samsung Galaxy Tab 8.9.

Navigate to http://www.wg9s.com/ in Landscape.

Rotate to portrait.
Build? Log?

I can't reproduce on my Galaxy Tab 10.1 (Android 3.1) w/ Nightly (01/20) nor Aurora (01/20).
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 719373
Not a dupe.  Bug 719373 is a Fennec Native bug.  This is Fennec XUL crash.
Status: RESOLVED → UNCONFIRMED
Ever confirmed: false
Resolution: DUPLICATE → ---
Status: UNCONFIRMED → NEW
Ever confirmed: true
I just verified that this issue is in the XUL build ONLY.
Summary: crash when rotating from landscape to portrait [@ TouchBadMemory] → Android XUL only crash when rotating from landscape to portrait [@ TouchBadMemory]
(In reply to Aaron Train [:aaronmt] from comment #1)
> Build? Log?
> 
> I can't reproduce on my Galaxy Tab 10.1 (Android 3.1) w/ Nightly (01/20) nor
> Aurora (01/20).

This is with the Android XUL official nightly located here:

http://ftp.mozilla.org/pub/mozilla.org/mobile/nightly/2012-01-20-03-11-25-mozilla-central-android-xul/fennec-12.0a1.multi.android-arm.apk
Whiteboard: [mobile-crash]
(In reply to Bill Gianopoulos [:WG9s] from comment #3)
> Not a dupe.  Bug 719373 is a Fennec Native bug.  This is Fennec XUL crash.
In addition, bug 719373 is a meta bug so each bug with STR should be marked as dependent.
Assignee: nobody → mbrubeck
tracking-fennec: --- → ?
Version: Trunk → Firefox 12
I am currently doing an hg bisect withing that window.
Bisect found this:

The first bad revision is:
changeset:   84713:2b834066b4ab
user:        Mounir Lamouri <mounir.lamouri@gmail.com>
date:        Mon Dec 19 11:16:39 2011 +0100
summary:     Bug 674725 - Part AB - Create a thread to handle SMS IO on Android. r=cjones
Blocks: websms
Building with "ac_add_options --disable-websms-backend" in .mozconfig is sufficient to avoid this crash.
This does not make a great deal of sense to me because:

1.  What does my website have to do with SMS?
2.  What does rotating the device have to do with SMS?
3.  Why does the same device not crash under native UI builds? (So, it can't just be blamed on some Honeycomb issue).

One thing to note is that SMS makes little sense on my device as it is WIFI only.  I am not sure if that fact has anything to do with why it crashes.
I'm seeing this on every orientation change on my TF101. As AFAIK we are still planning to ship the XUL version of 12 to tablet users, we really should look into this.
I can reproduce this bug and am looking into it.  (It happens on all my Android devices, not just Honeycomb tablets.)
It's #2 top crasher in Fennec 12.0a1.
Here's the stack trace from the logs.  The crash happens because rotating the device destroys and re-creates the activity.  When onCreate calls GeckoSmsManager.init a second time, it crashes trying to start a thread that is already running.

I/GeckoApp( 4538): pause
I/GeckoApp( 4538): stop
I/GeckoApp( 4538): destroy
I/GeckoSurfaceView( 4538): surface destroyed
I/GeckoApp( 4538): create
E/GeckoApp( 4538): top level exception
E/GeckoApp( 4538): java.lang.RuntimeException: Unable to start activity ComponentInfo{org.mozilla.fennec/org.mozilla.fennec.App}: java.lang.IllegalThreadStateException: Thread already started.
E/GeckoApp( 4538): 	at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:1651)
E/GeckoApp( 4538): 	at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:1667)
E/GeckoApp( 4538): 	at android.app.ActivityThread.handleRelaunchActivity(ActivityThread.java:2836)
E/GeckoApp( 4538): 	at android.app.ActivityThread.access$1600(ActivityThread.java:117)
E/GeckoApp( 4538): 	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:939)
E/GeckoApp( 4538): 	at android.os.Handler.dispatchMessage(Handler.java:99)
E/GeckoApp( 4538): 	at android.os.Looper.loop(Looper.java:130)
E/GeckoApp( 4538): 	at org.mozilla.gecko.GeckoApp$3.run(GeckoApp.java:356)
E/GeckoApp( 4538): 	at android.os.Handler.handleCallback(Handler.java:587)
E/GeckoApp( 4538): 	at android.os.Handler.dispatchMessage(Handler.java:92)
E/GeckoApp( 4538): 	at android.os.Looper.loop(Looper.java:130)
E/GeckoApp( 4538): 	at android.app.ActivityThread.main(ActivityThread.java:3687)
E/GeckoApp( 4538): 	at java.lang.reflect.Method.invokeNative(Native Method)
E/GeckoApp( 4538): 	at java.lang.reflect.Method.invoke(Method.java:507)
E/GeckoApp( 4538): 	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:842)
E/GeckoApp( 4538): 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:600)
E/GeckoApp( 4538): 	at dalvik.system.NativeStart.main(Native Method)
E/GeckoApp( 4538): Caused by: java.lang.IllegalThreadStateException: Thread already started.
E/GeckoApp( 4538): 	at java.lang.Thread.start(Thread.java:1227)
E/GeckoApp( 4538): 	at org.mozilla.gecko.GeckoSmsManager.init(GeckoSmsManager.java:367)
E/GeckoApp( 4538): 	at org.mozilla.gecko.GeckoApp.onCreate(GeckoApp.java:417)
E/GeckoApp( 4538): 	at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1047)
E/GeckoApp( 4538): 	at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:1615)
E/GeckoApp( 4538): 	... 16 more
D/Zygote  (   74): Process 4538 exited cleanly (1)
I/ActivityManager(  113): Process org.mozilla.fennec (pid 4538) has died.
This bug also affects Native fennec, though it is harder to reproduce there because native Fennec has configChanges="orientation" in the AndroidManifest for the main activity.  But native Fennec will crash in the same place if the OS locale, font size preference, number of screens, or other configurations change while it is running.
Attached patch XUL Fennec patchSplinter Review
This patch ensures that the SMS thread is started only once, and interrupted only when the activity finishes.  The intent receiver still needs to be added/removed whenever the activity is created/destroyed, since it is attached to the activity.
Attachment #590797 - Flags: review?(mounir)
Attachment #590797 - Flags: review?(blassey.bugs)
Crash Signature: [@ TouchBadMemory] → [@ TouchBadMemory] [@ GeckoSmsManager.init]
Summary: Android XUL only crash when rotating from landscape to portrait [@ TouchBadMemory] → Android XUL only crash when rotating from landscape to portrait [@ TouchBadMemory][@ GeckoSmsManager.init]
The same change applied to the native Fennec code.  Steps to reproduce the crash in native Fennec:

1. Open Fennec.
2. Open the Android "Settings" app.
3. In the "Language & keyboard" section, change the system language.
4. Return to Fennec.
Attachment #590811 - Flags: review?(mounir)
Attachment #590811 - Flags: review?(blassey.bugs)
Blocks: 719373
Whiteboard: [mobile-crash] → [mobile-crash][native-crash]
Attachment #590797 - Flags: review?(mounir) → review+
Attachment #590811 - Flags: review?(mounir) → review+
Summary: Android XUL only crash when rotating from landscape to portrait [@ TouchBadMemory][@ GeckoSmsManager.init] → SMS related Android crashes. Device rotation in XUL builds and language change in native builds [@ TouchBadMemory][@ GeckoSmsManager.init]
(In reply to Matt Brubeck (:mbrubeck) from comment #17)
> Created attachment 590797 [details] [diff] [review]
> XUL Fennec patch
> 
> This patch ensures that the SMS thread is started only once, and interrupted
> only when the activity finishes.  The intent receiver still needs to be
> added/removed whenever the activity is created/destroyed, since it is
> attached to the activity.

This seems to fix the issue that I originally reported for me.
Comment on attachment 590797 [details] [diff] [review]
XUL Fennec patch

Review of attachment 590797 [details] [diff] [review]:
-----------------------------------------------------------------

::: embedding/android/GeckoSmsManager.java
@@ -928,2 @@
>    public void shutdown() {
> -    GeckoApp.mAppContext.unregisterReceiver(this);

nit, call stop() instead
Attachment #590797 - Flags: review?(blassey.bugs) → review+
Comment on attachment 590811 [details] [diff] [review]
Native Fennec patch

Review of attachment 590811 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/GeckoApp.java
@@ +1931,5 @@
>  
>          if (SmsManager.getInstance() != null) {
> +            SmsManager.getInstance().stop();
> +            if (isFinishing())
> +                SmsManager.getInstance().shutdown();

the implementation of shutdown() does what stop() does, so only call one or the other:

if (isFinishing())
    SmsManager.getInstance().shutdown();
else
    SmsManager.getInstance().stop();

::: mobile/android/base/GeckoSmsManager.java
@@ -928,2 @@
>    public void shutdown() {
> -    GeckoApp.mAppContext.unregisterReceiver(this);

nit, call stop() instead
Attachment #590811 - Flags: review?(blassey.bugs) → review+
(In reply to Brad Lassey [:blassey] from comment #20)
> > -    GeckoApp.mAppContext.unregisterReceiver(this);
> 
> nit, call stop() instead

This line was removed by the patch.  Ignoring this comment, with Brad's permission via IRC.

https://hg.mozilla.org/integration/mozilla-inbound/rev/7cafa419af61
Status: NEW → ASSIGNED
Target Milestone: --- → Firefox 12
https://hg.mozilla.org/mozilla-central/rev/7cafa419af61
Status: ASSIGNED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
I can verify that rotation doesn't crash XUL trunk any more on my TF101.

The question is if bug 720734, bug 717663 and bug 719373 are the same thing or something else.
Status: RESOLVED → VERIFIED
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #24)
> The question is if bug 720734, bug 717663 and bug 719373 are the same thing
> or something else.
No. In FennecAndroid, crashes with exceptions are characterized by their exception in App notes, not by their crash signatures. Different crash signatures can be the same crash if they have the same exception in App Notes.
fyi, Kairo, scoobidiver is correct.
Hit return too quickly.  It will be changing where the Java Crashes will be moving to it's own field rather than in the App Notes for Socorro.  [bug 701390 and Bug 718820/ 721078]
Matt, this isn't a problem on Aurora, right? If so, please mark the whiteboard not-fennec-11
Whiteboard: [mobile-crash][native-crash] → [mobile-crash][native-crash][not-fennec-11]
Crash Signature: [@ TouchBadMemory] [@ GeckoSmsManager.init] → [@ TouchBadMemory] [@ TouchBadMemory | mozalloc_abort | dalvik-LinearAlloc (deleted)@0x16fe] [@ GeckoSmsManager.init]
tracking-fennec: ? → ---
You need to log in before you can comment on or make changes to this bug.