RTCIceGatherer.close affect on RTCIceTransport / RTCDtlsTransport #208

Closed
robin-raymond opened this Issue May 26, 2015 · 7 comments

Projects

None yet

4 participants

@robin-raymond
Contributor

needs to:

  • closes ports
  • moves associated RTCIceTransport to "disconnected" state
  • does not affect attached RTCDtlsTransport

(answer this issue on list, see if it is "good enough", think this is already all done in spec)

@aboba
Contributor
aboba commented May 26, 2015

Here is the description of iceGatherer.close():

close
Prunes all local candidates. Associated RTCIceTransport objects transition to the "disconnected" state. Calling close() when state is "closed" has no effect.

Calling iceGatherer.close() will also cause iceGatherer.state to transition to "closed" as noted in the description of the "closed" state:

closed
The RTCIceGatherer has been closed intentionally or as the result of an error.

The definition of the iceTransport.state value of "disconnected" is:

disconnected
The RTCIceTransport has received at least one local and remote candidate, and a local and remote RTCIceCandidateComplete dictionary was not added as the last candidate, but all appropriate candidate pairs thus far have been tested and failed (or consent checks, once successful, have now failed). Other candidate pairs may become available for testing as new candidates are trickled, and therefore the "failed" state has not been reached.

Here is the definition of dtlsTransport.state of "closed" :

closed
The DTLS connection has been closed intentionally via a call to stop() or as the result of an error (such as a failure to validate the remote fingerprint). Calling transport.stop() will also result in a transition to the "closed" state.

[BA] As a result of the above, if iceGatherer.close() is called, iceGatherer.state transitions to "closed" and the associated IceTransport objects transition to the '"disconnected" state. What if iceTransport.state had previously been "failed"? Should a transition to "disconnected" still occur? The proposed IceTransport state diagram seems to show a potential transition between "failed" and "disconnected", so it seems the answer is "yes".

However, dltsTransport.state does not appear to be affected, since calling iceGatherer.close() results in transport.state transitioning to "disconnected" and not "closed".

@aboba
Contributor
aboba commented Jun 3, 2015

Proposed update to definitions of "close" and "closed":

close
Prunes all local candidates, and closes the port. Associated RTCIceTransport objects transition to the "disconnected" state (unless they were in the "failed" state). Calling close() when state is "closed" has no effect.

closed
The RTCIceGatherer has been closed intentionally (by calling close()) or as the result of an error.

Updated definition of "getLocalCandidates"

Change:

"getLocalCandidates
Retrieve the sequence of valid local candidates associated with the RTCIceGatherer. This retrieves all local candidates currently known (except for peer reflexive candidates), even if an onlocalcandidate event hasn't been processed yet. Pruning of candidate pairs within an associated RTCIceTransport does not affect the candidates returned by getLocalCandidates()>."

To:

"getLocalCandidates
Retrieve the sequence of valid local candidates associated with the RTCIceGatherer. This retrieves all unpruned local candidates currently known (except for peer reflexive candidates), even if an onlocalcandidate event hasn't been processed yet."

@aboba
Contributor
aboba commented Jun 3, 2015

Also, here is a proposed revision in Section 13.1. Change:

"Gathers stats for the given object and reports the result asynchronously.

When the getStats() method is invoked, the user agent must queue a task to run the following steps:

For RTCDtlsTransport.getStats(), check whether RTCDtlsTransport.start() has been called; if not, throw an InvalidStateError exception. For RTCIceTransport.getStats(), check whether RTCIceTransport.start() has been called; if not, or if RTCIceTransport.stop() has been called, throw an InvalidStateError exception. For RTCRtpSender.getStats(), check whether RTCRtpSender.send(parameters) has been called; if not, throw an InvalidStateError exception. For RTCRtpReceiver.getStats(), check whether RTCRtpReceiver.receive(parameters) has been called; if not, throw an InvalidStateError exception."

to:

"getStats
Gathers stats for the given object and reports the result asynchronously.

When the getStats() method is invoked, the user agent must queue a task to run the following steps:

Let p be a new promise.

For RTCDtlsTransport.getStats(), check whether RTCDtlsTransport.state is "closed"; if so, reject p with an InvalidStateError. For RTCIceGatherer.getStats(), check whether RTCIceGatherer.state is "closed"; if so, reject p with an InvalidStateError. For RTCIceTransport.getStats(), check whether RTCIceTransport.state is "closed"; if so, reject p with an InvalidStateError."

@aboba aboba closed this Jun 16, 2015
@robin-raymond robin-raymond pushed a commit that referenced this issue Jun 22, 2015
Robin Raymond Addressed Philipp Hancke's review comments, as noted in: Issue #198
Added the "failed" state to RTCIceTransportState, as noted in: Issue #199
Added text relating to handling of incoming media packets prior to remote fingerprint verification, as noted in: Issue #200
Added a complete attribute to the RTCIceCandidateComplete dictionary, as noted in: Issue #207
Updated the description of RTCIceGatherer.close() and the "closed" state, as noted in: Issue #208
Updated Statistics API error handling to reflect proposed changes to the WebRTC 1.0 API, as noted in: Issue #214
Updated Section 10 (RTCDtmfSender) to reflect changes in the WebRTC 1.0 API, as noted in: Issue #215
Clarified state transitions due to consent failure, as noted in: Issue #216
Added a reference to [FEC], as noted in: Issue #217
894b08b
@juberti
juberti commented Jun 24, 2015

This proposal (.close causes IceTransport.disconnected) makes sense to me. However, I don't think failed -> disconnected makes sense.

Peter and I discussed what should happen to DtlsTransport state upon an IceTransport disconnection. I am leaning toward having disconnected on DtlsTransport, but we need to understand exactly what disconnected means in a DtlsTransport context.

@pthatcherg

This same question has come up while adding DtlsTransport to WebRTC 1.0: Should DtlsTransport have a "disconnected" state that it goes into when the IceTransport underneath is "disconnected"? If so, then IceGatherer.close should cause DtlsTransport to go into the disconnected state. If not, I think it should stay in the connected state (just as it would in normal ICE disconnects).

@robin-raymond
Contributor

@pthatcher I agree with you. I think it remains connected despite the transport underneath being disconnected since the ice transport disconnection could resolve itself with additional candidates and the dtls transport doesn't need to reassert the connected state after.

@pthatcherg

The tradeoff is the following is either:

A. Require the application to check both DtlsTransport.state and IceTransport.state to know if everything is connected. This requires more complexity in the app.

B. Require the DtlsTransport to know the IceTransport.state to change its state based on the IceTransport.state. This requires more complexity in the implementation.

I think if we only have the disconnected state, then I'm inclined to say go with B. But if it becomes a slippery slope where we have to have a lot of duplicate state, I'd prefer A.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment