com.unity.webrtc icon indicating copy to clipboard operation
com.unity.webrtc copied to clipboard

[BUG]: PCOnTrack & PCOnRemoveTrack bad endure GC collection -> weakReferences corrupted in table context

Open RiccardoCalcagno opened this issue 3 years ago • 5 comments

Package version

2.4.0-exp.5

Environment

OS: Windows 10 Pro
Unity version: 2020.3.23f1
From Editor

Steps To Reproduce

Environment: Application that manages 2 or more Peerconnections (the bug is present even with only two peers). There is a lot of asynchronicity, and many coroutines undergoing in the same thread (the main thread).

Configuration: Setted the PCOnTrack property of the PeerConnection with a body in wich there are:

  1. An access to a field (as the id) of the track arrived as an argument of the RTCTrackEvent.
  2. a call (at the end of the delegate) of the: GC.Collect(). (Here I'm forcing it. However, autonomous calls to GC collection could occur in this time).

Run:

  1. A remote new track reaches the WebRTC level (triggering the native delegate: DelegateNativeOnTrack).
  2. My access to the id of the track did not cause any error. I verified that checking with the debugger the content of the Context.table (without access to its runtime calculated properties) -> the library methods added properly the transceiver receiver and track to the table.
  3. Exiting the delegate, the execution returns to the PCOnTrack static method. Here, when the process tries to access again the track, shows up the following problem : The pointer of the RTCRtpReceiver related to the track is present in the table but it has no object (as Target) of type RTCRtpReceiver.

I think this means that before the second reading was made the GC Collector had altered the weakReferences of the receviver, setting its isAlive to false so now I have in the table a corrupted Receiver

Possible solution?? Couldn't we just remove the RefCountedObject objects from the table when the become not more alive?

Current Behavior

No response

Expected Behavior

No response

Anything else?

First relevant stack call in RTCPeerConnection.cs First relevant stack call

Second and Third relevant stack calls in RTCTrackEvent class of MediaStreamTrack.cs Second and Third relevant stack calls

Fourth relevant stack call in RTCRtpTransceiver.cs Fourth relevant stack call

Last failing stack call in WebRTC.cs Last failing stack call

RiccardoCalcagno avatar Feb 18 '22 15:02 RiccardoCalcagno

What do you think about the following solution?

In this static method: Last failing stack call we could change the first if condition Context.table.ContainsKey(ptr) with: Context.table.ContainsKey(ptr) && Context.table[ptr] != null because it become null when GC Collect erase its value trurning its IsAlive field in false

RiccardoCalcagno avatar Feb 18 '22 17:02 RiccardoCalcagno

@RiccardoCalcagno This story looks strange. GC collector call the finalizer of instances, and the finalizer should removes the item from the table.

karasusan avatar Feb 19 '22 02:02 karasusan

I know but after the GC.Collect() it still finds a weakReference inside the table with the same Prt, not Alive anymore. Might it be added a second time in a second moment? Can I please ask you If the error occurs even in your environment (with the configuration implying the GC.Collect) ?

RiccardoCalcagno avatar Feb 19 '22 08:02 RiccardoCalcagno

@RiccardoCalcagno I wrote this test. As you said, the issue is reproduced. Thanks. I am considering how to solve the issue.

        [UnityTest]
        [Timeout(5000)]
        public IEnumerator GCCollect()
        {
            GameObject obj = new GameObject("audio");
            AudioSource source = obj.AddComponent<AudioSource>();
            var test = new MonoBehaviourTest<SignalingPeers>();

            var track = new AudioStreamTrack(source);
            var sender = test.component.AddTrack(0, track);
            yield return test;
            GC.Collect();
            var receivers = test.component.GetPeerReceivers(1); // error! System.InvalidCastException : 2082804238160 is not RTCRtpReceiver
            Assert.That(receivers.Count(), Is.EqualTo(1));
            var receiver = receivers.First();
            var audioTrack = receiver.Track as AudioStreamTrack;
            Assert.That(audioTrack, Is.Not.Null);

            test.component.Dispose();
            UnityEngine.Object.DestroyImmediate(test.gameObject);
            UnityEngine.Object.DestroyImmediate(obj);
        }

karasusan avatar Feb 19 '22 09:02 karasusan

memo: WRS-320

karasusan avatar Apr 26 '22 02:04 karasusan