[reSIProcate] Treatment of CANCEL in UAS_Accepted or UAS_AcceptedWaitingAnswer
Scott Godin
sgodin at sipspectrum.com
Wed Sep 2 17:40:35 CDT 2009
I just realized that cancel responses are not routed to the
Dialog/DialogSet - I made a modification to the fix - try this one
instead:
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:
+ if (code / 100 == 1)
+ {
+ // do nothing - wait for final response
+ }
+ // 200/Inv crossing CANCEL case
+ else 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: Destruction of this dialog object will
cause DialogSet::possiblyDie to be called thus invoking mDum.destroy
+ }
+ else
+ {
+ 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 4:49 PM, Scott Godin<sgodin at sipspectrum.com> wrote:
> 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 at sipspectrum.com> 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 at yahoo.ca> 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 at sipspectrum.com> wrote:
>>>
>>>> From: Scott Godin <sgodin at sipspectrum.com>
>>>> Subject: Re: [reSIProcate] Treatment of CANCEL in UAS_Accepted or UAS_AcceptedWaitingAnswer
>>>> To: "Boris Rozinov" <borisrozinov at yahoo.ca>
>>>> Cc: resiprocate-devel at resiprocate.org
>>>> 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 at yahoo.ca>
>>>> 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 at resiprocate.org
>>>> > https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>>>> >
>>>>
>>>
>>>
>>> __________________________________________________________________
>>> Looking for the perfect gift? Give the gift of Flickr!
>>>
>>> http://www.flickr.com/gift/
>>>
>>
>
More information about the resiprocate-devel
mailing list