W3C home > Mailing lists > Public > public-webrtc-logs@w3.org > December 2019

[webrtc-pc] Tight races between ONN and onmessage in perfect negotiation (#2404)

From: Jan-Ivar Bruaroey via GitHub <sysbot+gh@w3.org>
Date: Tue, 10 Dec 2019 02:53:03 +0000
To: public-webrtc-logs@w3.org
Message-ID: <issues.opened-535459242-1575946382-sysbot+gh@w3.org>
jan-ivar has just created a new issue for https://github.com/w3c/webrtc-pc:

== Tight races between ONN and onmessage in perfect negotiation ==
I believe I've found some tight races in the perfect negotiation model between ONN and onmessage. They're not fatal, but may perhaps lead to confusing error noise, if not dealt with.

1. **ONN right before onmessage:** signalingState is async and insufficient to avoid collision. Solution: JS sets new "offering” boolean var in ONN and JS tests against it in onmessage in addition to signaling state.
1. **onmessage right before ONN.** signalingState is async and insufficient to avoid collision.
    Can’t move onmessage, which is timing sensitive (candidates come right after).
    Solution: modify spec to chain ONN on operations chain. 

With both the issue is that signalingState isn't synchronous, so relying on it to avoid collision between onmessage and ONN is insufficient in very close races. So to be 100% airtight JS needs another JS state variable (akin to the one I added earlier in this thread) to solve (1):
```js
pc.onnegotiationneeded = async () => {
  pc.offering = true; // <-- JS-created and stored state (on pc for convenience)
  await pc.setLocalDescription();
  send({description: desc});
};

signaling.onmessage = async ({data: {description, candidate}}) => {
  try {
    if (description) {
      if (!polite && description.type == "offer" && (pc.offering || pc.signalingState != "stable")) {
        ignoredRemoteDescription = true;
        return;
      }
      pc.offering = true
```
This takes care of the ONN right before onmessage.

The other one (2) I think it would be simple and reasonable to add a spec fix for (chain ONN). That way ONN won't happen immediately if methods are queued on the peer connection (which could be changing state on us). This seems like an improvement, and in keeping with intent, which I think was always to fire ONN when things have settled a bit.

Please view or discuss this issue at https://github.com/w3c/webrtc-pc/issues/2404 using your GitHub account
Received on Tuesday, 10 December 2019 02:53:05 UTC

This archive was generated by hypermail 2.4.0 : Friday, 17 January 2020 19:22:34 UTC