817841 - Race condition deleting nsDOMMediaStream via proxying to MainThread

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect)

Description

[Randell Jesup [:jesup] (needinfo me)]

We have a number of hard-to-reproduce reports of crashes due to a Release off-main-thread in ~MediaOperationRunnable(), which deletes a nsDOMMediaStream (which is a cycle-collected object, and thus main-thread only).

It tries to proxy to mainthread by resending itself there and letting the post-Run release kill it, but if MainThread runs *before* the off-main-thread Run() exists, it gets deleted off-main-thread (boom).

A better solution (though it means allocating) is to proxy to main thread just the Release() via NS_ProxyRelease().  This code isn't a performance hotspot so the alloc is not a big deal.

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Proxy deletion of nsDOMMediaStreams with NS_ProxyRelease()

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 688006 [details] [diff] [review]
Proxy deletion of nsDOMMediaStreams with NS_ProxyRelease()

Seems to work fine for me; I don't have a working reproducible case (though p.franc does I think) so I can't verify it fixes the problem directly.

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d9bcbec33260

Once baked we should consider uplift to Aurora

Comment 4

[Jason Duell]

backed out (f88a7f4b3792) for compiler error (warning->error: comma at end of enum list) and relanded: (I hope that's acceptable procedure: wanted to make sure we don't repeat same problem on aurora/beta if we copy patch to them)

  https://hg.mozilla.org/integration/mozilla-inbound/rev/857e8fdd2d5d

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

As per IRC: thanks

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 02/17 - 02/21)]

Looks like bug 817976 is a dupe, right?

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/857e8fdd2d5d

Comment 8

[Jason Smith [:jsmith]]

comment 2 implies there isn't a reproducible test case. If there is one, feel free to comment to explain it.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 02/17 - 02/21)]

Test cases from crash reports are:

http://mozilla.github.com/webrtc-landing/gum_test.html
http://alexandre.alapetite.fr/doc-alex/html5-webcam/
https://apprtc.appspot.com/
http://mozilla.github.com/webrtc-landing/pc_test.html

Comment 11

[Jason Smith [:jsmith]]

Sounds like the best course of action here is probably then is to only reopen if we reproduce this crash again. It looks like the URLs in comment 10 imply that general gum usage could reproduce this.

Comment 12

[Pavel Franc - Mozilla.cz]

I can not reproduce this particular crash anymore with current nightly.

Comment 13

[Jason Smith [:jsmith]]

Already fixed at this point, no need to get a reduced test case here.