1807700 - Follow gc rules in webrtc-pc

Status: NEW

(Core :: WebRTC: Signaling, task, P3)

Description

[Byron Campen [:bwc]]

webrtc-pc says:

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

All RTCDataChannel and MediaStreamTrack objects that are connected to an RTCPeerConnection have a strong reference to the RTCPeerConnection object."

At first glance, I don't think we follow this. We need to check:

1. Having an event handler registered on a PC prevents it from being garbage-collected.
2. Holding a reference to a MediaStreamTrack is sufficient to keep a PC alive.
3. Holding a reference to an RTCDataChannel is sufficient to keep a PC alive.

Having wpt for these things would be ideal.

Comment 1

[Andreas Pehrson [:pehrsons]]

(In reply to Byron Campen [:bwc] from comment #0)

1. Having an event handler registered on a PC prevents it from being garbage-collected.

I assume we could reach a state where we know that we will not fire any more events?

2. Holding a reference to a MediaStreamTrack is sufficient to keep a PC alive.

MediaStreamTrackSource is cycle collecting, but its subclass RemoteTrackSource is not. It should be, just like CanvasCaptureTrackSource.

I think this would fix bug 1805317 and bug 1805320 because the DOM holds a ref to the media element, which holds a ref to the MediaStream which holds a ref to the MediaStreamTrack which holds a ref to the MediaStreamTrackSource, which in this case does not hold a ref to any of the peer connection objects per above.

Comment 2

[Byron Campen [:bwc]]

(In reply to Andreas Pehrson [:pehrsons] from comment #1)

(In reply to Byron Campen [:bwc] from comment #0)

1. Having an event handler registered on a PC prevents it from being garbage-collected.

I assume we could reach a state where we know that we will not fire any more events?

Closing the PC would do that, but there are not many cases otherwise. Maybe if you did a negotiation that tore down all the transports?

2. Holding a reference to a MediaStreamTrack is sufficient to keep a PC alive.

MediaStreamTrackSource is cycle collecting, but its subclass RemoteTrackSource is not. It should be, just like CanvasCaptureTrackSource.

I think this would fix bug 1805317 and bug 1805320 because the DOM holds a ref to the media element, which holds a ref to the MediaStream which holds a ref to the MediaStreamTrack which holds a ref to the MediaStreamTrackSource, which in this case does not hold a ref to any of the peer connection objects per above.

Could be. Couldn't hurt to try.