Bug 235167 - MediaStreamTrack endedEvent fired when RTCPeerConnection connectionState is already RTCPeerConnectionState::Closed
Summary: MediaStreamTrack endedEvent fired when RTCPeerConnection connectionState is a...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
Keywords: InRadar
Depends on:
Reported: 2022-01-12 17:11 PST by Gabriel Nava Marino
Modified: 2022-01-19 17:12 PST (History)
2 users (show)

See Also:


Note You need to log in before you can comment on or make changes to this bug.
Description Gabriel Nava Marino 2022-01-12 17:11:07 PST
This bug is to investigate whether the current RTCPeerConnection follows from the W3 recommendation mentioned in https://www.w3.org/TR/webrtc/#garbage-collection.

"An RTCPeerConnection object MUST not be garbage collected as long as any event can cause an event handler to be triggered on the object. When the object's [[IsClosed]] internal slot is true, no such event handler can be triggered and it is therefore safe to garbage collect the object."

Bug 235165 shows there are some scenarios that allow the event handler to be triggered on endedEvent, after the RTCPeerConnection "object's [[IsClosed]] internal slot is true".

More specifically, this can happen when stopping RTCPeerConnection from the destructor. In this case, RTCPeerConnection::doClose() sets the connectionState to RTCPeerConnectionState::Closed, but without preventing the endedEvent from firing while stopping the RTCRtpReceiver (RTCRtpTransceiver).
Comment 1 youenn fablet 2022-01-12 22:50:42 PST
I think all browsers do fire ended events.
I am guessing we should either update the spec or implementations.
Comment 2 youenn fablet 2022-01-12 22:59:01 PST
Ah, nevermind, browsers do fire events when close is called.
I think they do not when their context goes away as the dom object would be stopped.
It would be good to validate that other browsers are not firing such events in case of GCing the peer connection.
Comment 3 Gabriel Nava Marino 2022-01-13 10:53:14 PST
Per Youenn's recommendation in the related radar:

We should probably update MediaStreamTrack::trackEnded to queue a task to fire the event instead of firing the event right away.
This matches https://w3c.github.io/mediacapture-main/#track-ended.
This would allow to remove the activeDOMObjectsAreSuspended/activeDOMObjectsAreStopped checks in that method.
It would also be good to write a corresponding test. internals.simulateMediaStreamTrackCaptureSourceFailure maybe be able to help validate that the event is not fired synchronously.
Comment 4 youenn fablet 2022-01-14 04:42:19 PST
Will implement "queue a task to fire ended event" in bug 235227.
Comment 5 Radar WebKit Bug Importer 2022-01-19 17:12:17 PST