Closed Bug 1236380 Opened 4 years ago Closed 4 years ago

[GMP] read@0x0 in GMPStorageParent::RecvGetRecordNames

Categories

(Core :: Audio/Video: GMP, defect, P1)

43 Branch
All
Windows
defect

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: gerald, Assigned: gerald)

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

https://crash-stats.mozilla.com/signature/?signature=mozilla%3A%3Agmp%3A%3AGMPStorageParent%3A%3ARecvGetRecordNames#reports

Getting 30+ reports per week, crashing in mozilla::gmp::GMPStorageParent::RecvGetRecordNames. All on Windows, most when accessing Netflix.

The crash site and exception point at 'mStorage' being NULL when dereferenced, which should theoretically not happen thanks to the |if (mShutdown) {return}| test above.
Maybe a race between Shutdown() and RecvGetRecordNames()?
Chris, Anthony -- This looks to be in your area of GMP.  Can one of you take it -- or find someone to take it (getting 30+ crash reports per week)?  Thanks.
Rank: 15
Flags: needinfo?(cpearce)
Flags: needinfo?(ajones)
Priority: -- → P1
(In reply to Maire Reavy [:mreavy] (Plz ni?:mreavy) from comment #1)
> Chris, Anthony -- This looks to be in your area of GMP.  Can one of you take
> it -- or find someone to take it (getting 30+ crash reports per week)? 
> Thanks.

I was planning to look into it in the next few days -- That's why I created the bug and assigned it to myself.

Chris: If you have the time, feel free to look into it, you would probably be quicker to assess the issue.
Flags: needinfo?(cpearce)
Sorry, Gerald, I didn't notice you assigned it to yourself.  I thought you were just reporting it.  Thanks for taking it!
Flags: needinfo?(ajones)
No worries Maire, I should have been more explicit in the description.
Also, in my comment 2 I intended to keep NI:Chris around (instead of Anthony), to benefit from his wisdom if time permits.
Flags: needinfo?(cpearce)
gerald: My guess would be that the child process created the GMPStorageChild, and since we don't block waiting for the connection to establish, the child process immediately called EnumerateRecords, and in the parent process GMPStorageParent::Init() failed before setting GMPStorageParent::mStorage, so when GMPStorageParent::RecvGetRecordNames() runs mShutdown is false but mStorage is null.
Flags: needinfo?(cpearce)
Initialize GMPStorage::mShutdown to true, so that if Init() has not completed
yet or if it failed, other methods will not try and access a null mStorage.

Review commit: https://reviewboard.mozilla.org/r/29681/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/29681/
Attachment #8704475 - Flags: review?(cpearce)
Attachment #8704475 - Flags: review?(cpearce) → review+
Comment on attachment 8704475 [details]
MozReview Request: Bug 1236380 - GMPStorage::mShutdown=true until Init() succeeds - r?cpearce

https://reviewboard.mozilla.org/r/29681/#review26583
Chris: Thank you for the hint, and then review.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ecf677579257
https://hg.mozilla.org/mozilla-central/rev/3ee42c6c0e25
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.