Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

INVITE without SDP and 200/ACK retransmission produces "Called in wrong state: STATE_INPROGRESS" #367

Closed
ibc opened this issue Apr 6, 2016 · 5 comments
Assignees
Labels
bug

Comments

@ibc
Copy link
Member

@ibc ibc commented Apr 6, 2016

Scenario:

  • INVITE received without SDP.
  • JsSIP answers 200 OK with a SDP offer.
  • ACK not received.
  • JsSIP retransmits the 200 OK with SDP offer.
  • Peer sends ACK with SDP answer and it is given to the PeerConnection.
  • A few milliseconds later the same ACK is received again (due our double 200). JsSIP gives it again to the PeerConnection, producing an error that ends the call:

ef2fAgent.js:48667 rtcninja:ERROR:RTCPeerConnection setRemoteDescription() | error: +0ms Failed to set remote answer sdp: Called in wrong state: STATE_INPROGRESS

Basically we must prevent that a retransmission of the ACK (with SDP answer in this scenario) breaks the underlying PeerConnection state.

@ibc ibc self-assigned this Apr 6, 2016
@ibc ibc added the bug label Apr 6, 2016
@ibc

This comment has been minimized.

Copy link
Member Author

@ibc ibc commented Apr 6, 2016

Also, in the above scenario, JsSIP does not send BYE (which it should).

@ibc

This comment has been minimized.

Copy link
Member Author

@ibc ibc commented Apr 6, 2016

The problem is here:

case JsSIP_C.ACK:
  if(this.status === C.STATUS_WAITING_FOR_ACK) {
    clearTimeout(this.timers.ackTimer);
    clearTimeout(this.timers.invite2xxTimer);

    if (this.late_sdp) {
      if (!request.body) {
        ended.call(this, 'remote', request, JsSIP_C.causes.MISSING_SDP);
        break;
      }

      this.connection.setRemoteDescription(
        new rtcninja.RTCSessionDescription({type:'answer', sdp:request.body}),
        // success
        function() {
          self.status = C.STATUS_CONFIRMED;
        },
        // failure
        function() {
          ended.call(self, 'remote', request, JsSIP_C.causes.BAD_MEDIA_DESCRIPTION);
        }
      );
    }
    else {
      this.status = C.STATUS_CONFIRMED;
    }

    if (this.status === C.STATUS_CONFIRMED && !this.is_confirmed) {
      confirmed.call(this, 'remote', request);
    }
  }
  break;

When the first ACK with SDP answer is received, this.status remains STATUS_WAITING_FOR_ACK after setRemoteDescription() succeeds. But it takes a bit so the second ACK is received in between, producing a second call to setRemoteDescription().

@jmillan may update this.status to STATUS_CONFIRMED upon receipt of the ACK? At the end such a status means "signaling status" and it should not depend on the SDP stuff.

Thoughts?

P.S. I don't understand yet why no BYE is generated.

ibc added a commit that referenced this issue Apr 6, 2016
@ibc

This comment has been minimized.

Copy link
Member Author

@ibc ibc commented Apr 6, 2016

ACK retransmissions are now properly handled (7790ffd). However since BYE is not sent I leave the issue open.

To be clear: If somehow the ACK processing fails, no BYE is sent (unless I'm missing something).

@jmillan

This comment has been minimized.

Copy link
Member

@jmillan jmillan commented Apr 6, 2016

@ibc, you're right. We were emitting the ended event but not sending the BYE. Would you please try, whenever possible, this patch as you already seem to have the scenario ready to test?

diff --git a/lib/RTCSession.js b/lib/RTCSession.js
index e1f2f81..7706033 100644
--- a/lib/RTCSession.js
+++ b/lib/RTCSession.js
@@ -1225,7 +1225,10 @@ RTCSession.prototype.receiveRequest = function(request) {

         if (this.late_sdp) {
           if (!request.body) {
-            ended.call(this, 'remote', request, JsSIP_C.causes.MISSING_SDP);
+            this.terminate({
+              cause: JsSIP_C.causes.MISSING_SDP,
+              status_code: 400
+            });
             break;
           }

@@ -1239,7 +1242,10 @@ RTCSession.prototype.receiveRequest = function(request) {
             },
             // failure
             function() {
-              ended.call(self, 'remote', request, JsSIP_C.causes.BAD_MEDIA_DESCRIPTION);
+              self.terminate({
+                cause: JsSIP_C.causes.BAD_MEDIA_DESCRIPTION,
+                status_code: 488
+              });
             }
           );
         }
@ibc ibc closed this in 68f0af8 Apr 6, 2016
@ibc

This comment has been minimized.

Copy link
Member Author

@ibc ibc commented Apr 6, 2016

Yep! Thanks.

0.7.22 published.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.