< Previous by Date Date Index Next by Date >
< Previous in Thread Thread Index Next in Thread >

Re: [reSIProcate] Treatment of CANCEL in UAS_Accepted or UAS_AcceptedWaitingAnswer


Here is a patch you can try out against SVN head.  I haven't had any
chance to test it myself, but let me know if it works for you:

Index: DialogSet.cxx
===================================================================
--- DialogSet.cxx       (revision 8559)
+++ DialogSet.cxx       (working copy)
@@ -239,7 +239,10 @@
 bool
 DialogSet::handledByAuthOrRedirect(const SipMessage& msg)
 {
-   if (msg.isResponse() && !(mState == Terminating || mState ==
WaitingToEnd || mState == Destroying))
+   if (msg.isResponse() && !(mState == Terminating ||
+                                mState == WaitingToEnd ||
+                                                        mState == Destroying ||
+                                                        mState == Cancelling))
    {
       // !dcm! -- multiple usage grief...only one of each method type allowed
       if (getCreator() &&
@@ -447,7 +450,52 @@
       }
       return;
    }
+   else if(mState == Cancelling)
+   {
+      assert(mDialogs.empty());
+      if (msg.isResponse())
+      {
+         int code = msg.header(h_StatusLine).statusCode();
+         switch(mCreator->getLastRequest()->header(h_CSeq).method())
+         {
+            case INVITE:
+                          // 200/Inv crossing CANCEL case
+               if (code / 100 == 2)
+               {
+                  Dialog dialog(mDum, msg, *this);

+                  SharedPtr<SipMessage> ack(new SipMessage);
+                  dialog.makeRequest(*ack, ACK);
+                  ack->header(h_CSeq).sequence() =
msg.header(h_CSeq).sequence();
+                  dialog.send(ack);
+
+                  SharedPtr<SipMessage> bye(new SipMessage);
+                  dialog.makeRequest(*bye, BYE);
+                  dialog.send(bye);
+
+                  // Note:  Normally Destruction of this dialog
object will cause DialogSet::possiblyDie to be called thus invoking
mDum.destroy
+                                 //        but we don't want the dialogset to 
go away until the
cancel response is received.  Setting mReUseDialogSet
+                                 //        will cause DialogSet::possiblyDie 
to not be called in
the Dialog destructor
+                                 dialog.mReUseDialogSet = true;
+               }
+               break;
+
+                       case CANCEL:
+               // Destroy dialog set when cancel response is received
+               mState = Destroying;
+               mDum.destroy(this);
+               break;
+         }
+      }
+      else
+      {
+         SharedPtr<SipMessage> response(new SipMessage);
+         mDum.makeResponse(*response, msg, 481);
+         mDum.send(response);
+      }
+      return;
+   }
+
    if (handledByAuthOrRedirect(msg))
    {
       return;
@@ -903,26 +951,17 @@
          SharedPtr<SipMessage>
cancel(Helper::makeCancel(*getCreator()->getLastRequest()));
          mDum.send(cancel);

+         if (mDum.mDialogEventStateManager)
+         {
+            mDum.mDialogEventStateManager->onTerminated(*this,
*cancel, InviteSessionHandler::LocalCancel);
+         }
+
          if (mDialogs.empty())
          {
-            // !jf! if 200/INV crosses a CANCEL that was sent after receiving
-            // non-dialog creating provisional (e.g. 100), then we need to:
-            // Add a new state, if we receive a 200/INV in this state, ACK and
-            // then send a BYE and destroy the dialogset.
-            if (mDum.mDialogEventStateManager)
-            {
-               mDum.mDialogEventStateManager->onTerminated(*this,
*cancel, InviteSessionHandler::LocalCancel);
-            }
-            mState = Destroying;
-            mDum.destroy(this);
+            mState = Cancelling;
          }
          else
          {
-            if (mDum.mDialogEventStateManager)
-            {
-               mDum.mDialogEventStateManager->onTerminated(*this,
*cancel, InviteSessionHandler::LocalCancel);
-            }
-
             //need to lag and do last element ouside of look as this
DialogSet will be
             //deleted if all dialogs are destroyed
             for (DialogMap::iterator it = mDialogs.begin(); it !=
mDialogs.end(); it++)
@@ -956,6 +995,7 @@
          break;
       }
       case Terminating:
+         case Cancelling:
       case Destroying:
          DebugLog (<< "DialogSet::end() called on a DialogSet that is
already Terminating");
          //assert(0);
Index: DialogSet.hxx
===================================================================
--- DialogSet.hxx       (revision 8559)
+++ DialogSet.hxx       (working copy)
@@ -69,6 +69,7 @@
          ReceivedProvisional,
          Established,
          Terminating,
+                Cancelling,  // only used when cancelling and no dialogs exist
          Destroying
       } State;


Scott

On Wed, Sep 2, 2009 at 12:05 PM, Scott Godin<sgodin@xxxxxxxxxxxxxxx> wrote:
> Yes I agree - that is a real issue in DUM, we need do something like
> keep the DialogSet around (after destruction/cancel) for some period
> of time (likely 32s) in order to correctly handle scenarios like this.
>
> Scott
>
> On Tue, Sep 1, 2009 at 5:45 PM, Boris Rozinov<borisrozinov@xxxxxxxx> wrote:
>> Scott,
>>
>> I believe you are right and UAS behaves correctly by just responding 200OK 
>> to Cancel. It brings us to UAC side (in our case both UAC and UAS are 
>> implemented using resiprocate).  UAC was supposed to wait for 200OK/INV and 
>> then respond by ACK and then send BYE. But DialogSet::end() sends CANCEL and 
>> in absence of created dialogs invokes mDum.destroy(this) and effectively 
>> destruct itself and as result delayed (or retransmitted) 200OK/INV are 
>> thrown away as stray response.
>>
>> Thanks,
>> Boris
>>
>>
>> --- On Tue, 9/1/09, Scott Godin <sgodin@xxxxxxxxxxxxxxx> wrote:
>>
>>> From: Scott Godin <sgodin@xxxxxxxxxxxxxxx>
>>> Subject: Re: [reSIProcate] Treatment of CANCEL in UAS_Accepted or 
>>>  UAS_AcceptedWaitingAnswer
>>> To: "Boris Rozinov" <borisrozinov@xxxxxxxx>
>>> Cc: resiprocate-devel@xxxxxxxxxxxxxxx
>>> Received: Tuesday, September 1, 2009, 1:22 PM
>>> I believe DUM is behaving according
>>> to RFC3261 - section 9.2:
>>>
>>> If the transaction
>>>    for the original request still exists,
>>> the behavior of the UAS on
>>>    receiving a CANCEL request depends on
>>> whether it has already sent a
>>>    final response for the original
>>> request.  If it has, the CANCEL
>>>    request has no effect on the processing
>>> of the original request, no
>>>    effect on any session state, and no
>>> effect on the responses generated
>>>    for the original request.
>>>
>>> Scott
>>>
>>> On Mon, Aug 31, 2009 at 11:52 PM, Boris Rozinov<borisrozinov@xxxxxxxx>
>>> wrote:
>>> > Hi all,
>>> >
>>> > Due to race condition UAS may receive CANCEL for the
>>> initial INVITE transaction in Accepted or
>>> AcceptedWaitingAnswer state. As transition to OnCancel state
>>> occurs, 200 OK response for Cancel is sent, but session is
>>> not terminated and session handler is not notified.
>>> > Does not it make more sense to send BYE (dialog is
>>> already established), terminate session and notify session
>>> handler (or just dispatch Cancel treatment to
>>> InviteSession)?
>>> >
>>> > Thanks,
>>> > Boris
>>> >
>>> >
>>> >
>>>  __________________________________________________________________
>>> > Yahoo! Canada Toolbar: Search from anywhere on the
>>> web, and bookmark your favourite sites. Download it now
>>> > http://ca.toolbar.yahoo.com.
>>> > _______________________________________________
>>> > resiprocate-devel mailing list
>>> > resiprocate-devel@xxxxxxxxxxxxxxx
>>> > https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>>> >
>>>
>>
>>
>>      __________________________________________________________________
>> Looking for the perfect gift? Give the gift of Flickr!
>>
>> http://www.flickr.com/gift/
>>
>