Yes – I can see that possibility if copies were possible – but that’s
not the case. Something else must be happening here.
From: Aron Rosenberg
[mailto:arosenberg@xxxxxxxxxxxxxx]
Sent: Sunday, February 03, 2008 5:27 PM
To: Scott Godin
Cc: resiprocate-devel@xxxxxxxxxxxxxxx
Subject: RE: [reSIProcate] Patch: Adds support to queue NITs
I don’t have the call stack handy, but the InviteSession “copy”
would try and delete our new NITqueue and it would have already been deleted in
the original instance so you would get a double delete/free bug. Making the
NITqueue a queue of auto_pointers fixed the whole issue in the patch.
-Aron
From: Scott Godin
[mailto:slgodin@xxxxxxxxxxxx]
Sent: Sunday, February 03, 2008 2:24 PM
To: Aron Rosenberg
Cc: resiprocate-devel@xxxxxxxxxxxxxxx
Subject: RE: [reSIProcate] Patch: Adds support to queue NITs
Why would it crash in the destructor?
Scott
From: Aron Rosenberg
[mailto:arosenberg@xxxxxxxxxxxxxx]
Sent: February 2, 2008 12:15 PM
To: Scott Godin
Cc: resiprocate-devel@xxxxxxxxxxxxxxx
Subject: RE: [reSIProcate] Patch: Adds support to queue NITs
Not sure where it would be copied, but we would get a crash in
the InviteSession destructor when resiprocate would shutdown if it wasn’t an
auto_pointer
From:
resiprocate-devel-bounces@xxxxxxxxxxxxxxx
[mailto:resiprocate-devel-bounces@xxxxxxxxxxxxxxx] On Behalf Of Scott
Godin
Sent: Friday, February 01, 2008 10:06 PM
To: Kobi Eshun
Cc: resiprocate-devel@xxxxxxxxxxxxxxx
Subject: Re: [reSIProcate] Patch: Adds support to queue NITs
When is an InviteSession ever copied? I don’t think this
is a problem.
From: Kobi Eshun
[mailto:kobi@xxxxxxxxxxxxxx]
Sent: February-01-08 6:50 PM
To: Scott Godin
Cc: resiprocate-devel@xxxxxxxxxxxxxxx
Subject: Re: [reSIProcate] Patch: Adds support to queue NITs
Hi Scott,
I found an issue with the implementation:
the elements of mNITQueue are not correctly duplicated on an InviteSession copy
as currently implemented (my bad, not yours).
I changed it to be a queue of shared rather
than raw pointers.
On Feb 1, 2008, at 9:07 AM, Scott Godin
wrote:
Thanks Kobi! I think this patch is
a good idea - I've had to do the same thing at the app level in many of the
applications I've created.
I've taken a look at the patch and have made some modifications to my local
copy - I'll give others a few days to digest and make comments, then I'll
commit it. Here are the changes I've made so far:
1. Removed use of the resip Fifo to store the queued NITs and replaced this
with an STL queue - the FIFO classes uses Mutex's that are just unneeded
overhead.
2. In an attempt to make things a little cleaner. I renamed the checkNITQueue
fn to nitComplete - this fn now sets the state to completed and checks the
queue. This new fn is called in dispatchInfo, dispatchMessage on responses and
on refer responses.
3. As a consequence of 2 - removed the checkNITQueue call from Dialog.cxx, when
receiving a response to INFO or MESSAGE NITs.
I've attached a new version of the patch.
Thanks,
Scott
> -----Original Message-----
> From: resiprocate-devel-bounces@xxxxxxxxxxxxxxx
[mailto:resiprocate-
> devel-bounces@xxxxxxxxxxxxxxx]
On Behalf Of Kobi Eshun
> Sent: Thursday, January 31, 2008 7:56 PM
> To: resiprocate-devel@xxxxxxxxxxxxxxx
> Subject: [reSIProcate] Patch: Adds support to queue NITs
>
> Hi,
>
> Please consider the attached patch against HEAD. It implements queueing
> for any outgoing REFER, INFO, or MESSAGE requests associated with an
> InviteSession. If a non-INVITE transaction is already in progress when
> refer(), info() or message() is invoked, the resulting SIP request is
> pushed into a FIFO. Queued requestes are shipped out one at a time
> without overlap as final responses are processed.
>
> For INFO and MESSAGE, the new behavior is semantically equivalent to
> before (queueing notwithstanding).
>
> For REFER, the previous code locked out a new request until the
> onReferAccepted() callback fired on the first NOTIFY. The patch changes
> the end of that lockout to coincide with the final response for the
> REFER itself. The new lockout period should be shorter, and proved
> easier to code. Does anyone see a problem with this?
>
> The new queue should also work with the DUM command API, but I did not
> test it.
>
> Cheers,
> --
> kobi
>
<resiprocate_queued_NITs-scott.diff>
|