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

Status: RESOLVED FIXED

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

Description

[Gerald Squelart [:gerald] (he/him)]

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()?

Comment 1

[Maire Reavy [:mreavy]]

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.

Comment 2

[Gerald Squelart [:gerald] (he/him)]

(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.

Comment 3

[Maire Reavy [:mreavy]]

Sorry, Gerald, I didn't notice you assigned it to yourself.  I thought you were just reporting it.  Thanks for taking it!

Comment 4

[Gerald Squelart [:gerald] (he/him)]

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.

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

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.

Comment 6

[Gerald Squelart [:gerald] (he/him)]

Attachment: MozReview Request: Bug 1236380 - GMPStorage::mShutdown=true until Init() succeeds - r?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/

Comment 7

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8704475 [details]
MozReview Request: Bug 1236380 - GMPStorage::mShutdown=true until Init() succeeds - r?cpearce

https://reviewboard.mozilla.org/r/29681/#review26583

Comment 8

[Gerald Squelart [:gerald] (he/him)]

Chris: Thank you for the hint, and then review.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ecf677579257

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3ee42c6c0e25

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3ee42c6c0e25