Closed
Bug 1270689
Opened 8 years ago
Closed 8 years ago
Widevine crash in non-virtual thunk to mozilla::WidevineDecryptor::OnSessionClosed when using DASH.js player
Categories
(Core :: Audio/Video: Playback, defect, P1)
Tracking
()
VERIFIED
FIXED
mozilla49
Tracking | Status | |
---|---|---|
firefox46 | --- | unaffected |
firefox47 | --- | verified |
firefox48 | --- | verified |
firefox49 | --- | verified |
People
(Reporter: cpeterson, Assigned: cpearce)
References
Details
(Keywords: crash)
Crash Data
Attachments
(2 files)
58 bytes,
text/x-review-board-request
|
mozbugz
:
review+
ritu
:
approval-mozilla-aurora+
ritu
:
approval-mozilla-beta+
|
Details |
1.35 KB,
patch
|
mozbugz
:
review+
ritu
:
approval-mozilla-aurora+
ritu
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is report bp-dcc4aefa-c837-490a-b026-aadbc2160505. ============================================================= STR from bug 1268694: 1. Load http://dashif.org/reference/players/javascript/v2.1.1/samples/dash-if-reference-player/index.html 2. Load manifest http://html5.cablelabs.com:8100/cenc/wv/dash.mpd (also accessible from the DASH player's menu: Sample Streams > CableLabs Test Content > CableLabs Cenc Widevine) on DASH.js player menu. 3. Quickly hit the "Load" button 2–3 more times. If the crash happens, it always seems to happen within the first 1–2 button clicks when you first play the video after the browser starts. 0 XUL non-virtual thunk to mozilla::WidevineDecryptor::OnSessionClosed(char const*, unsigned int) dom/media/gmp/widevine-adapter/WidevineDecryptor.cpp:396 Ø 1 libwidevinecdm.dylib libwidevinecdm.dylib@0x4f88 Ø 2 libwidevinecdm.dylib libwidevinecdm.dylib@0x1bddf Ø 3 libwidevinecdm.dylib libwidevinecdm.dylib@0x2615e Ø 4 libwidevinecdm.dylib libwidevinecdm.dylib@0x18677 Ø 5 libwidevinecdm.dylib libwidevinecdm.dylib@0x1bf50 6 XUL mozilla::gmp::GMPRunnable::Run() dom/media/gmp/GMPPlatform.cpp:40 7 XUL RunnableMethod<mozilla::gmp::GMPRunnable, void (mozilla::gmp::GMPRunnable::*)(), mozilla::Tuple<> >::Run ipc/chromium/src/base/task.h:28 8 XUL MessageLoop::DoWork() ipc/chromium/src/base/message_loop.cc:335 9 XUL base::MessagePumpDefault::Run(base::MessagePump::Delegate*) ipc/chromium/src/base/message_pump_default.cc:34 10 XUL MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:233 11 XUL XRE_InitChildProcess toolkit/xre/nsEmbedFunctions.cpp:637 12 plugin-container content_process_main(int, char**) ipc/contentproc/plugin-container.cpp:237 13 plugin-container start
Assignee | ||
Comment 1•8 years ago
|
||
I can't repro this, but I suspect it could happen if we try to shutdown the CDM before we've finished starting it up. We can probably just add a null check here.
Assignee | ||
Comment 2•8 years ago
|
||
We've observed some crashes derefing the callback pointer, which may be occuring due to shutdown happening before init has setup the callback pointer. Review commit: https://reviewboard.mozilla.org/r/51303/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/51303/
Attachment #8750139 -
Flags: review?(gsquelart)
Comment on attachment 8750139 [details] MozReview Request: Bug 1270689 - Null-check WidevineDecryptor::mCallback before use. r=gerald https://reviewboard.mozilla.org/r/51303/#review47969 r+ with this fixed: ::: dom/media/gmp/widevine-adapter/WidevineDecryptor.cpp:318 (Diff revision 1) > + Log("Decryptor::OnRejectPromise(aPromiseId=%d, err=%d, sysCode=%d, msg=%s) FAIL; !mCallback", > + aPromiseId, (int)aError, aSystemCode, aErrorMessage); 'syscode=%u'
Attachment #8750139 -
Flags: review?(gsquelart) → review+
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → cpearce
Assignee | ||
Comment 4•8 years ago
|
||
Comment on attachment 8750139 [details] MozReview Request: Bug 1270689 - Null-check WidevineDecryptor::mCallback before use. r=gerald Review request updated; see interdiff: https://reviewboard.mozilla.org/r/51303/diff/1-2/
Attachment #8750139 -
Attachment description: MozReview Request: Bug 1270689 - Null-check WidevineDecryptor::mCallback before use. r?gerald → MozReview Request: Bug 1270689 - Null-check WidevineDecryptor::mCallback before use. r=gerald
Comment 7•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b2d509c083a9
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Assignee | ||
Comment 8•8 years ago
|
||
Comment on attachment 8750139 [details] MozReview Request: Bug 1270689 - Null-check WidevineDecryptor::mCallback before use. r=gerald Approval Request Comment [Feature/regressing bug #]: Widevine EME [User impact if declined]: Widevine EME video may fail to play. Under some unknown circumstances, the Widevine CDM is crashing. I think this happens when we try to shutdown before we've completed startup (so failing to play may actually not be noticed). We only have a handful of crash reports, so it should be rare. [Describe test coverage new/current, TreeHerder]: We have lots of EME mochitests. [Risks and why]: Low; we're just adding null checks. [String/UUID change made/needed]: None.
Attachment #8750139 -
Flags: approval-mozilla-beta?
Attachment #8750139 -
Flags: approval-mozilla-aurora?
Comment on attachment 8750139 [details] MozReview Request: Bug 1270689 - Null-check WidevineDecryptor::mCallback before use. r=gerald I only saw one instance of this crash in the past 7 days and that was on Nightly49, however I am still taking this patch as it puts some safeguards in place, Aurora48+, Beta47+
Attachment #8750139 -
Flags: approval-mozilla-beta?
Attachment #8750139 -
Flags: approval-mozilla-beta+
Attachment #8750139 -
Flags: approval-mozilla-aurora?
Attachment #8750139 -
Flags: approval-mozilla-aurora+
Comment 10•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/1bb8e2207e8e
Comment 11•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/47f753b5632b
Comment 12•8 years ago
|
||
On Mac OS X 10.11 - Nightly 49 and Firefox 47 beta 5 are still crashing for me. Here are the crash reports: https://crash-stats.mozilla.com/report/index/9d207049-6012-4dc0-b5eb-6e7fc2160513 https://crash-stats.mozilla.com/report/index/4d41abad-acad-4a39-8f30-60f732160513 Here are the STR: 1. Open Firefox 2. Start playing several amazon videos (I played 7 videos) 3. Close Firefox by clicking on the X button from the upper left side of the window 4. In the Dock bar click on the Firefox icon to open a new window 5. Navigate to about:crashes Actual results: A new crash report is added to the Submitted crash reports list. Chris, should I Reopen this bug or file a new one due to the fact the scenario is different.
Reporter | ||
Comment 13•8 years ago
|
||
Let's just reopen this bug. When I filed this bug, we knew we had crash reports for both DASH.js and Amazon. So these crashes are all the same bug.
Assignee | ||
Comment 14•8 years ago
|
||
The OnSessionClosed() callback is happening on a timer, after WidevineDecryptor::DecryptingComplete() has been called. So mCallback could be non-null, be actually because DecryptingComplete() has been called, the object pointed to be mCallback has been deallocated, and mCallback is a dangling pointer. MozReview-Commit-ID: 4xdHYRn7EAS
Attachment #8752640 -
Flags: review?(gsquelart)
Comment on attachment 8752640 [details] [diff] [review] Clear WidevineDecryptor::mCallback in WidevineDecryptor::DecryptingComplete() Review of attachment 8752640 [details] [diff] [review]: ----------------------------------------------------------------- r+, but please revisit your commit description: 'could be non-null, be actually because' -> 'could actually be non-null because' (assuming that's what you meant), 'pointed to be mCallback' -> 'pointed to by mCallback'.
Attachment #8752640 -
Flags: review?(gsquelart) → review+
Assignee | ||
Comment 16•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/30f9e7dfcd3d3de1d949f523d353d9956f2b976a Bug 1270689 - Clear WidevineDecryptor::mCallback in WidevineDecryptor::DecryptingComplete(). r=gerald
Assignee | ||
Comment 17•8 years ago
|
||
(In reply to Simona B [:simonab] from comment #12) > On Mac OS X 10.11 - Nightly 49 and Firefox 47 beta 5 are still crashing for > me. > > Here are the crash reports: > https://crash-stats.mozilla.com/report/index/9d207049-6012-4dc0-b5eb- > 6e7fc2160513 > https://crash-stats.mozilla.com/report/index/4d41abad-acad-4a39-8f30- > 60f732160513 > > Here are the STR: > 1. Open Firefox > 2. Start playing several amazon videos (I played 7 videos) > 3. Close Firefox by clicking on the X button from the upper left side of the > window > 4. In the Dock bar click on the Firefox icon to open a new window > 5. Navigate to about:crashes > > Actual results: > A new crash report is added to the Submitted crash reports list. > > Chris, should I Reopen this bug or file a new one due to the fact the > scenario is different. Thanks Simona, I was able to repro and debug this using these STR. :)
Assignee | ||
Comment 18•8 years ago
|
||
Comment on attachment 8752640 [details] [diff] [review] Clear WidevineDecryptor::mCallback in WidevineDecryptor::DecryptingComplete() Approval Request Comment [Feature/regressing bug #]: Widevine EME [User impact if declined]: Crashes during shutdown if Widevine EME video is playing; the impact on users is very low, since typically the crash isn't observable, but we will receive crash reports for these crashes, so we should eliminate the crash, otherwise crashes with the same signature would be missed. [Describe test coverage new/current, TreeHerder]: We have lots of EME mochitests [Risks and why]: Low; annulling a pointer on shutdown, and we always null-check that pointer before using it (added in the previous patch in this bug). [String/UUID change made/needed]: None.
Attachment #8752640 -
Flags: approval-mozilla-beta?
Attachment #8752640 -
Flags: approval-mozilla-aurora?
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(cpearce)
Priority: -- → P1
Comment 19•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/30f9e7dfcd3d
Status: REOPENED → RESOLVED
Closed: 8 years ago → 8 years ago
Resolution: --- → FIXED
Comment on attachment 8752640 [details] [diff] [review] Clear WidevineDecryptor::mCallback in WidevineDecryptor::DecryptingComplete() Crash fix, Aurora48+, Beta47+
Attachment #8752640 -
Flags: approval-mozilla-beta?
Attachment #8752640 -
Flags: approval-mozilla-beta+
Attachment #8752640 -
Flags: approval-mozilla-aurora?
Attachment #8752640 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 21•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/63933fe5bb3f
Assignee | ||
Comment 22•8 years ago
|
||
ni? me; I need to remember to land the aurora uplift.
Flags: needinfo?(cpearce)
Assignee | ||
Comment 23•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/9ba18aae69e6
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(cpearce)
Comment 24•8 years ago
|
||
I can confirm the issue is fixed on the latest Nightly 49.0a1 (Build ID: 20160519030232)and Firefox 47 beta 7 (Build ID: 20160518173344) - neither of this Firefox versions are crashing when following the STR from Comment 12 on Max OS X 10.11
Comment 25•8 years ago
|
||
Verified as fixed also Firefox Developer Edition 48.0a2 (Build:20160522004024) - there is no crash with the signature [@ non-virtual thunk to mozilla::WidevineDecryptor::OnSessionClosed ]- when following the scenarios from the Description and from Comment 12. However, I did encountered a new crash when playing on the Dashif webpage, filed Bug 1274963 to cover that.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•