1891503 - WebRTC ICE candidates gathering issue TurnTLS

Status: UNCONFIRMED

(Core :: WebRTC, defect)

Description

[k-wasniowski]

Attachment: firefox-failure.png

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/123.0.0.0 Safari/537.36

Steps to reproduce:

1. Enter WebRTC sample: https://webrtc.github.io/samples/src/content/peerconnection/trickle-ice
2. Clear all ICE Servers
3. Enter the turn tls server with data:

• URI - turns:freeturn.net:5349
• TURN username: free
• TURN password: free

4. Set IceTransport as 'relay'
5. Press Gather candidates

I do not have any VPN or Proxy server configured

Actual results:

Sometimes no candidates are returned

Expected results:

To always gather some candidates

Comment 1

[k-wasniowski]

Attachment: firefox-success.png

Comment 2

[k-wasniowski]

Attached a screenshots with the view of my outcome. Compared it to different browser, but encountered this issue only on firefox. Also tested with different TURN servers, but the issue is still occurring.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::WebRTC' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 4

[Michael Froman [:mjf]]

I can confirm this behavior - Safari lets me bang away on "Gather candidates", while after a few times in Firefox, we start getting no candidates. Waiting a bit seems to allow it gather once, and then it will fail again.

Byron, since you've been looking ice stuff lately, would you be able to take a look at this?

Comment 5

[Michael Froman [:mjf]]

Marking S3 for now - Byron, if you decide it is more critical feel free to update.

Comment 6

[Byron Campen [:bwc]]

So, I can reproduce this, but when I look at the logs, we're gathering (and trickling) the relay candidate just fine. I suspect that the JS is somehow missing it. Looking closer.

Comment 7

[Byron Campen [:bwc]]

Ok, based on the code, I have a hypothesis for what is going on. Inside the candidate handler, there is this:

    if (['srflx', 'relay'].includes(candidate.type) && !candidate.url) {
      const stats = await pc.getStats();
      stats.forEach(report => {
        if (!url && report.type === 'local-candidate' &&
            report.address === candidate.address &&
            report.port === candidate.port) {
          url = report.url;
        }
      });
    }

This blocks the addition of the candidate to the table until a stats query completes. This means that the gathering complete callback can happen before the stats query resolves:

  if (pc.iceGatheringState !== 'complete') {
    return;
  }
  const elapsed = ((window.performance.now() - begin) / 1000).toFixed(3);
  const row = document.createElement('tr');
  appendCell(row, elapsed);
  appendCell(row, getFinalResult());
  pc.close();
  pc = null;
  if (stream) {
    stream.getTracks().forEach(track => track.stop());
    stream = null;
  }
  gatherButton.disabled = false;
  getUserMediaInput.disabled = false;
  candidateTBody.appendChild(row);
}

That code closes the PC, ensuring that the stats query never finishes, so the candidate is never added to the table.

Comment 8

[Byron Campen [:bwc]]

Ok, I'm definitely seeing that the getStats call is not resolving sometimes (in JS), which is preventing candidates from being added to the table. However, on the c++ side we're definitely resolving those same getStats calls, albeit after the PC is closed. There seems to be some sort of breakdown with promise resolution here.

Comment 9

[Byron Campen [:bwc]]

Jan-Ivar, is there some subtlety that could explain what is happening here? JS is calling getStats inside pc.onicecandidate, then adapter.js calls the "real" getStats, then JS closes the PC in pc.onicegatheringstatechange, then on the c++ side we're resolving both the MozPromise and then the dom::Promise (and making it all the way to enqueuing runnables in CycleCollectedJSContext::DispatchToMicroTask), but the getStats call that adapter.js made never resolves (runs its .then function)?

Comment 10

[Byron Campen [:bwc]]

This kinda smells like another JSImpl bug...