1154873 - GMPDecryptorParent::Shutdown() sends a message from the wrong thread

Status: RESOLVED INCOMPLETE

(Core :: Audio/Video, defect)

Description

[Andrew McCreight [:mccr8]]

I found this crash signature on crash-stats:
https://crash-stats.mozilla.com/report/index/bf30f18e-654f-4289-af2c-69a3d2150414

GMPDecryptorParent::Shutdown() is run off the main thread, but calls SendDecryptingComplete(), which calls MessageChannel::Send(), which I think is not okay to do on this random thread.

Comment 1

[Andrew McCreight [:mccr8]]

Two more like this:
https://crash-stats.mozilla.com/report/index/31c719b8-42a6-4137-8343-e2faf2150414
https://crash-stats.mozilla.com/report/index/9ea68906-0f6e-4b64-a8d8-f00132150414

This one is slightly different, in that it is GMPVideoDecoderParent::Shutdown() calling SendDecodingComplete() off the main thread, but it seems like the same basic problem:
https://crash-stats.mozilla.com/report/index/0897ed68-e6b5-4576-93cf-9eb012150414#allthreads

The last one has the user comment "Testing GMP crash reporting... Crashes browser :(".

Comment 2

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

(In reply to Andrew McCreight [:mccr8] from comment #0)
> GMPDecryptorParent::Shutdown() is run off the main thread, but calls
> SendDecryptingComplete(), which calls MessageChannel::Send(), which I think
> is not okay to do on this random thread.

If I understand correctly, PGMP is meant to be owned by a non-main thread — notice that CDMProxy::Shutdown asserts it's on the main thread and then sends a runnable to mGMPThread to call CDMProxy::gmp_Shutdown, and this design pattern is repeated for a few other methods.  The problem here is that it somehow winds up on a thread that isn't the owner of its MessageChannel.

Comment 3

[Andrew McCreight [:mccr8]]

Seems hard to exploit...

Comment 4

[Andrew McCreight [:mccr8]]

I haven't seen this in a while.