Closed Bug 1425898 Opened 2 years ago Closed 2 years ago

Crash in nsObserverService::NotifyObservers

Categories

(Core :: Internationalization, defect, critical)

Unspecified
Android
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox-esr52 --- unaffected
firefox57 --- unaffected
firefox58 --- fixed
firefox59 --- fixed

People

(Reporter: marcia, Assigned: droeh)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is
report bp-24cd2ffb-824b-4e7c-9d41-d47590171201.
=============================================================

Seen while looking at nightly crashes - 70 crashes/50 installs - crashes started using 20171130101255: http://bit.ly/2BtC3cD. Crash also present on 58, starting in Beta 9.

A few comments point to the same cause:

*Crashed after changing the phone language from Japanese to US English. 
*Switched phone language. 

Top 10 frames of crashing thread:

0 libxul.so nsObserverService::NotifyObservers xpcom/ds/nsObserverService.cpp:284
1 libxul.so mozilla::intl::OSPreferences::Refresh intl/locale/OSPreferences.cpp:82
2 libdvm.so libdvm.so@0x1e74e 
3 dalvik-heap (deleted) dalvik-heap @0x8e626 
4 dalvik-heap (deleted) dalvik-heap @0x1fb19e 
5 libdvm.so libdvm.so@0x4fb73 
6 data@app@org.mozilla.fennec_aurora-1.apk@classes.dex data@app@org.mozilla.fennec_aurora-1.apk@classes.dex@0x5ab747 
7 libxul.so mozilla::jni::Context<mozilla::java::BrowserLocaleManager, _jobject*>::ClassRef widget/android/jni/Refs.h
8 libdvm.so libdvm.so@0xbf93e 
9 dalvik-heap (deleted) dalvik-heap @0xaace26 

=============================================================
Crash occurs on Android APIs from 27 (Oreo) all the way to 17 (Jelly Bean).
ni on Ioana to try to reproduce this issue - it happens on latest Pixel devices as well as the original Pixel devices.
Flags: needinfo?(ioana.chiorean)
Any ideas, Gandalf? It looks like you added this observer service call in bug 1308329.
Flags: needinfo?(gandalf)
I believe the only change in this area recently was in bug 1337078. :jchen or :snorp should be able to verify if this seems related.
Flags: needinfo?(snorp)
Flags: needinfo?(nchen)
Flags: needinfo?(gandalf)
Looks like we're calling `OSPreferences::Refresh` on the UI thread but should be calling it on the Gecko thread.
Flags: needinfo?(snorp)
Flags: needinfo?(nchen)
Flags: needinfo?(droeh)
Yeah, my bad.
Assignee: nobody → droeh
Flags: needinfo?(droeh)
Attachment #8938163 - Flags: review?(nchen)
Comment on attachment 8938163 [details] [diff] [review]
Call refreshLocales on the Gecko thread rather than the UI thread

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

::: mobile/android/base/java/org/mozilla/gecko/BrowserLocaleManager.java
@@ +469,1 @@
>      public static native void refreshLocales();

Might as well change it to "private" while you're at it.
Attachment #8938163 - Flags: review?(nchen) → review+
Pushed by droeh@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6357003f9a47
Call refreshLocales on the Gecko thread rather than the UI thread. r=jchen
https://hg.mozilla.org/mozilla-central/rev/6357003f9a47
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
the signature is spiking up in volume during recent 58.0b builds on fennec - can we get the patch uplifted there too?
Flags: needinfo?(droeh)
Comment on attachment 8938163 [details] [diff] [review]
Call refreshLocales on the Gecko thread rather than the UI thread

Approval Request Comment
[Feature/Bug causing the regression]:1337078
[User impact if declined]:Potential crashes
[Is this code covered by automated tests?]:No
[Has the fix been verified in Nightly?]:Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]:None
[Is the change risky?]:No
[Why is the change risky/not risky?]:Just ensures a call gets dispatched to the correct thread.
[String changes made/needed]:None
Flags: needinfo?(droeh)
Attachment #8938163 - Flags: approval-mozilla-beta?
Comment on attachment 8938163 [details] [diff] [review]
Call refreshLocales on the Gecko thread rather than the UI thread

Fix a crash. Beta58+.
Attachment #8938163 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Tried with 58.0b14 build 2 on the following devices: 
- Pixel 2 Android 8.1.0 (even if I do not see them for the last days at least http://bit.ly/2qtT58W)
- Xiaomi Redmi Note 3 ( saw Redmi 4X in the report but we don't have it)
but I was not able to reproduce it at all. 

Steps: 
- change the phone language from Japanese to US English and back
- change the phone language from Romanian/Espanol (es-US) to US English and back

As this latest Beta gets to our users we should try to see if we get new reports in. Will keep an eye on it.
Flags: needinfo?(ioana.chiorean)
You need to log in before you can comment on or make changes to this bug.