Mixedreality-webrtc: PeerConnection.Initialized set too early

[Dr-Internet]

Describe the bug
Description of PeerConnetion.InitializeAsync states that: "Once this call asynchronously completed, the Initialized property becomes true.". This seems not to be working.

To Reproduce
Consider following function:

async void foo(){
             Console.WriteLine("is initialized1 " + Peer.Initialized);
            await Peer.InitializeAsync(config);
            Console.WriteLine("is initialized2 " + Peer.Initialized);
}

then call it

{
           foo();
           Console.WriteLine("is initialized3 " + Peer.Initialized);
}

The messages it produces are:

is initialized1 False
is initialized3 True
is initialized2 True

Expected behavior
Since "is initialized3" is called before InitializeAsync finishes, it should be False.

is initialized1 False
is initialized3 False
is initialized2 True

Environment

• MR-WebRTC version: 2.0.2
• Platform: Win10
• Architecture: x64
• Unity version (if applicable): non-Unity
• Target device: Desktop

[djee-ms]

Alright so this is awkward but we shouldn't have a PeerConnection.Initialized in the first place. I thought this was helpful but looking more closely it is confusing and cannot be implemented as it was originally envisioned.

The reason is that we do not control the change of state of the task, .NET does. And therefore we cannot possibly synchronize the changes of PeerConnection.Initialized. Even if we change PeerConnection.Initialized = true at the very end of the task, there is always a race condition where PeerConnection.Initialized = true but Task.IsCompleted is not true yet because .NET is in the process of changing that state but didn't quite yet.

The conclusion is that we should remove PeerConnection.Initialized and encourage users instead to await/Wait() the PeerConnection.InitializeAsync() task and have their own bool set after that if they want/need to for their own application logic.

[djee-ms]

PeerConnection.Initialized is now marked as obsolete and the docs have been updated to add a warning not to use it, and instead solely rely on the completion of the async task of PeerConnection.InitializeAsync() to avoid any confusion.