Re: [reSIProcate] Treatment of CANCEL in UAS_Accepted or UAS_AcceptedWaitingAnswer
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@xxxxxxxxxxxxxxx> 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@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/
>>>
>>
>