Deadlocks when using Dispose/Close PeerConnection on UI Thread

[AtosNicoS]

Describe the bug
I had some deadlock issues with the library when trying to dispose the PeerConnection.

To Reproduce
Steps to reproduce the behavior:

1. Close The Peer Connection on the UI Thread
2. The Close call will block, unless all events are finished
3. Some events are waiting to switch to the main thread, because they want to pause the media player
4. Deadlock. the UI thread waits on the events (for Close()), the events wait to get the ui thread.

Environment
Please fill the information for each peer if different

• Platform: UWP
• Architecture: x64
• Target device: Windows Desktop
• Version 1.0.3 nuget

Additional context
The official example does not show how to dispose/close a call. How can we ensure to not dispose the PeerConnection on the ui thread without using Task.Run and using an async dispose? Async Dispose is something I want to avoid.

I can workaround this issue by disabling the frame updates and stopping the remote video before calling Close():

private void RemoveRemoteStream()
        {
            _peerConnection.I420RemoteVideoFrameReady -= Peer_RemoteI420FrameReady;

            // Just double-check that the remote video track is indeed playing
            // before scheduling a task to stop it. Currently the remote video
            // playback is exclusively controlled by the remote track being present
            // or not, so these should be always in sync.
            lock (_isRemoteVideoPlayingLock)
            {
                if (_isRemoteVideoPlaying)
                {
                    // Schedule on the main UI thread to access STA objects.
                    _ = CoreApplication.MainView.CoreWindow.Dispatcher.RunTaskAsync(CoreDispatcherPriority.Normal, () =>
            {
                // Check that the remote video is still playing.
                // This ensures that rapid calls to add/remove the video track
                // are serialized, and an earlier remove call doesn't remove the
                // track added by a later call, due to delays in scheduling the task.
                lock (_isRemoteVideoPlayingLock)
                {
                    if (_isRemoteVideoPlaying)
                    {
                        _remoteVideoPlayer.Pause();
                        _remoteVideoPlayer.Source = null;
                        _isRemoteVideoPlaying = false;
                    }
                }
            });
                }
            }
        }

[astaikos316]

I am having a similar issue with not being able to close a call and try to start a new one using the master branch. When trying to start a new call I am getting an index out of bounds exception PeerConnection.StartConnection(), the Debug.Assert(transceivers[index++] == tr); is throwing the exception.

[astaikos316]

@djee-ms is there any possibility the transceivers are not getting cleaned up properly after a close() and then trying to start a new connection?

[djee-ms]

@astaikos316 > Yes it's likely, thanks for opening #375.

[fibann]

@AtosNicoS I couldn't reproduce this on latest master (I tried adding a Close() call on the UI thread to the test UWP app). Can you verify if this still happens to you, and in that case provide a code sample?

[astaikos316]

@fibann in relation to this as i posted in #375 have you seen that behaviour? It is still happening for me.

[AtosNicoS]

@fibann I am using 1.0.3 as this is the current stable. I do not have the time resources to test the current master, as it has breaking API changes it seems. If version 2.0 comes out, we will integrate the upgrade and I am able to test. However the issue from @astaikos316 seems to exists as well?

[fibann]

@AtosNicoS preview packages for 2.0 are available at https://github.com/microsoft/MixedReality-WebRTC/releases/tag/v2.0.0-preview.1, we hope to finalize the release in the next few days. There are no further work/fixes planned to 1.0, so our recommendation is switching to 2.0 as soon as it is convenient.

#375 has been resolved (also, it probably doesn't affect 1.0).

[AtosNicoS]

From what I've noticed, that deadlock issue is still there. But I do not have a minimal example to test, sadly.

[fibann]

@AtosNicoS I have been double-checking the original post:

Some events are waiting to switch to the main thread, because they want to pause the media player

From this line it sounds like it's app code (in an event callback) that wants to switch to the main thread, rather than MR-WebRTC internal code. Can you confirm? If this is the case, then this is not a bug, but an unavoidable side effect of WebRTC serializing all API calls and callbacks on its signaling thread. So you can't call any WebRTC API if you are blocking the signaling thread by e.g. waiting in an event callback.