1425898 - Crash in nsObserverService::NotifyObservers

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[Marcia Knous [:marcia]]

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@[e-mail][e-mail] data@[e-mail][e-mail]@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 

=============================================================

Comment 1

[Marcia Knous [:marcia]]

Crash occurs on Android APIs from 27 (Oreo) all the way to 17 (Jelly Bean).

Comment 2

[Marcia Knous [:marcia]]

ni on Ioana to try to reproduce this issue - it happens on latest Pixel devices as well as the original Pixel devices.

Comment 3

[Andrew McCreight (out until 2026) [:mccr8]]

Any ideas, Gandalf? It looks like you added this observer service call in bug 1308329.

Comment 4

[Zibi Braniecki [:zbraniecki][: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.

Comment 5

[(inactive) Jim Chen [:jchen] [:darchons]]

Looks like we're calling `OSPreferences::Refresh` on the UI thread but should be calling it on the Gecko thread.

Comment 6

[Dylan Roeh (:droeh) (he/him)]

Attachment: Call refreshLocales on the Gecko thread rather than the UI thread

Yeah, my bad.

Comment 7

[(inactive) Jim Chen [:jchen] [:darchons]]

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.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6357003f9a47
Call refreshLocales on the Gecko thread rather than the UI thread. r=jchen

Comment 9

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/6357003f9a47

Comment 10

[[:philipp]]

the signature is spiking up in volume during recent 58.0b builds on fennec - can we get the patch uplifted there too?

Comment 11

[Dylan Roeh (:droeh) (he/him)]

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

Comment 12

[Gerry Chang [:gchang]]

Comment on attachment 8938163 [details] [diff] [review]
Call refreshLocales on the Gecko thread rather than the UI thread

Fix a crash. Beta58+.

Comment 13

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/releases/mozilla-beta/rev/24cedcea08dc

Comment 14

[Ioana Chiorean]

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.