Removing DataChannels causes InvalidOperationException

[torepaulsson]

Describe the bug When calling PeerConnection.RemoveDataChannel an InvalidOperationException is thrown.

To Reproduce Steps to reproduce the behavior:

1. Go to 'TestNetCoreConsole' project
2. Add the following code after the pc.Initialize, after line 49 in Program.cs

   Console.WriteLine("Adding data channel.");
   await pc.AddDataChannelAsync("the_data_channel", true, true, CancellationToken.None);

3. Add the following code when stopping the recording, after line 128 in Program.cs

   Console.WriteLine("Removing data channels...");
   foreach (var dataChannel in pc.DataChannels.ToImmutableArray())
   {
     pc.RemoveDataChannel(dataChannel);
   }

4. See error thrown is caught in the general exception handler, line ~143.
5. Stack trace shows:

   at System.ThrowHelper.ThrowInvalidOperationException_HandleIsNotInitialized()
   at System.Runtime.InteropServices.GCHandle.FromIntPtr(IntPtr value)
   at Microsoft.MixedReality.WebRTC.Interop.Utils.ReleaseWrapperRef(IntPtr wrapperRef) in C:\code\MixedReality-WebRTC\libs\Microsoft.MixedReality.WebRTC\Interop\InteropUtils.cs:line 135
   at Microsoft.MixedReality.WebRTC.DataChannel.DestroyNative() in C:\code\MixedReality-WebRTC\libs\Microsoft.MixedReality.WebRTC\DataChannel.cs:line 210
   at Microsoft.MixedReality.WebRTC.PeerConnection.RemoveDataChannel(DataChannel dataChannel) in C:\code\MixedReality-WebRTC\libs\Microsoft.MixedReality.WebRTC\PeerConnection.cs:line 1476
   at TestNetCoreConsole.Program.<Main>d__0.MoveNext() in C:\code\MixedReality-WebRTC\examples\TestNetCoreConsole\Program.cs:line 133

Expected behavior Data channel connection removed and resources cleared from memory

Environment

• MR-WebRTC version (package version, or specific commit if building from source): master
• Platform: Windows Desktop
• Architecture: x64
• Target device: Windows Desktop

Additional context I noticed this issue when I was using data channels and wanted to remove them from the peer connection. It always crashed, thought I would try to reproduce it in a easier context.

From what I can tell while debugging, it seems that the RemoveDataChannel method removes the channel from the list, calls the c++ implementation of peer_connection.RemoveDataChannel, it performs some work and fires an DataChannelRemoved callback. A lot of work is done in these callback handlers, it seems some work is duplicated, especially the dataChannel.DestroyNative() method call.

As the code looks now it seems like you probably would get multiple RemoveDataChannel events if you initialize the removal, but when the callback from the mr-webrtc library is called we also destroy the data channel. I believe we should not call destroy native in the PeerConnection.RemoveDataChannel, probably only do that in the PeerConnectionInterop.DataChannelRemovedCallback call?

I've created a branch where I've investigated the issue, and tried to fix it, https://github.com/microsoft/MixedReality-WebRTC/compare/master...torepaulsson:users/torep/fix-remove-data-channel

[wenjunche]

Hello

I am seeing the same issue with RemoveDataChannel:

Environment: MR-WebRTC: Latest stable 2.0.2 Platform: Windows 10.0.19042 Build 199042 Architecture: x64 Target device: Windows Desktop

Thanks

[djee-ms]

Fixed with @torepaulsson's PR #799, thanks for the contribution! Closing this.