Closed
Bug 1384578
Opened 7 years ago
Closed 7 years ago
Crash in java.lang.NullPointerException: Null native pointer at org.mozilla.gecko.media.GeckoHLSDemuxerWrapper$Callbacks.onError(Native Method)
Categories
(Firefox for Android Graveyard :: Audio/Video, defect)
Tracking
(firefox54 unaffected, firefox55 unaffected, firefox56 fixed)
RESOLVED
FIXED
Firefox 56
Tracking | Status | |
---|---|---|
firefox54 | --- | unaffected |
firefox55 | --- | unaffected |
firefox56 | --- | fixed |
People
(Reporter: calixte, Assigned: JamesCheng)
References
Details
(Keywords: crash)
Crash Data
Attachments
(1 file)
This bug was filed from the Socorro interface and is report bp-18d49337-7eda-47fb-b9b0-720340170726. ============================================================= There are 2 crashes in nightly 56 with buildid 20170719100212 and 20170724100320. :JamesCheng, could you investigate please ?
Flags: needinfo?(jacheng)
Assignee | ||
Comment 1•7 years ago
|
||
Thanks for reporting, investigating!
Assignee | ||
Comment 2•7 years ago
|
||
My guess is There is a race condition between [1] http://searchfox.org/mozilla-central/rev/09c065976fd4f18d4ad764d7cb4bbc684bf56714/dom/media/hls/HLSDemuxer.cpp#255-263 and [2] http://searchfox.org/mozilla-central/rev/09c065976fd4f18d4ad764d7cb4bbc684bf56714/mobile/android/geckoview/src/main/java/org/mozilla/gecko/media/GeckoHlsPlayer.java#353-357 So [2] might access the callback which has been destroyed by [1]. I need to find a synchronization approach between java and cpp. Hi Jim, May I have your opinions that is there any existing code handling the synchronization between java and cpp? I can only think of exposing some @WrapForJNI APIs from java like 'Lock() / Unlock()' and use it in C++ side... Thanks.
Flags: needinfo?(nchen)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 4•7 years ago
|
||
As the assumption in comment 2, [1] and [2] are racy. I think the patch is a simple way to make them mutual exclusive. synchronized void GeckoPlayer.onPlayerError and public synchronized void GeckoPlayer.release()(which is called by mHLSDemuxerWrapper->Destroy()) is mutual exclusive by 'synchronized' So the only thing we need to adjust is that making HLSDemuxerCallbacksSupport::DisposeNative(mJavaCallbacks) be the last call to avoid this calling sequence 1. HLSDemuxerCallbacksSupport::DisposeNative(mJavaCallbacks) 2. GeckoPlayer.onPlayerError 3. GeckoPlayer.release() always make it 1, 2. GeckoPlayer.onPlayerError | GeckoPlayer.release() 3. HLSDemuxerCallbacksSupport::DisposeNative(mJavaCallbacks) [1] http://searchfox.org/mozilla-central/rev/09c065976fd4f18d4ad764d7cb4bbc684bf56714/dom/media/hls/HLSDemuxer.cpp#255-263 and [2] http://searchfox.org/mozilla-central/rev/09c065976fd4f18d4ad764d7cb4bbc684bf56714/mobile/android/geckoview/src/main/java/org/mozilla/gecko/media/GeckoHlsPlayer.java#353-357
Assignee | ||
Updated•7 years ago
|
Attachment #8891872 -
Flags: review?(jolin)
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8891872 [details] Bug 1384578 - Adjust the calling sequence to avoid app crash by race condition. https://reviewboard.mozilla.org/r/162890/#review168210
Attachment #8891872 -
Flags: review?(jolin) → review+
Assignee | ||
Comment 6•7 years ago
|
||
Thank you for review!
Pushed by jacheng@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/116f553ba4cd Adjust the calling sequence to avoid app crash by race condition. r=jolin
Comment 8•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/116f553ba4cd
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Comment 9•7 years ago
|
||
You can lock Java objects in C++ through `jni::Ref::Lock`, e.g., > { > auto lock = mJavaObject.Lock(); > // foo > } is similar to this Java code, > synchronized (mJavaObject) { > // foo > } But your solution looks good!
Flags: needinfo?(nchen)
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•