[reSIProcate] Patch: Adds support to queue NITs

Kobi Eshun kobi at sightspeed.com
Tue Feb 5 00:06:33 CST 2008 

Yup, that would do it. I'm fine with either solution too. Cheers,
--
kobi


On Feb 4, 2008, at 2:27 PM, Scott Godin wrote:

> Ah – I think I see what’s happening.
>
> The code in the destructor is just wrong:
>
> QueuedNIT *qn;
> while (qn=mNITQueue.front())
> {
>     mNITQueue.pop();
>     delete qn;
> }
>
> Should be re-written to be:
>
> while (mNITQueue.size())
> {
>     delete mNITQueue.front();
>     mNITQueue.pop();
> }
>
> mNITQueue.front() will return an undefined value if queue is empty.
>
> The auto_ptr solution is a reasonable solution as well.
>
> Scott
>
> From: Kobi Eshun [mailto:kobi at sightspeed.com]
> Sent: Monday, February 04, 2008 1:32 AM
> To: Scott Godin
> Cc: resiprocate-devel at resiprocate.org
> Subject: Re: [reSIProcate] Patch: Adds support to queue NITs
>
> You're right about the copying -- I see that the InviteSession copy  
> constructors are in fact disabled.
>
> But there's definitely something rattling around in there. Here's a  
> stack trace from a reliable crash from a test application. (Ignore  
> the apparently different reSIProcate header/source paths -- they're  
> a coherent set.) The app establishes a call with a UAS, fires off a  
> bunch of in-dialog NITs, and then tears down the call. It crashes  
> while destroying the InviteSession.
>
> Replacing the raw pointers in the NIT queue with SharedPtr's fixes  
> the problem, but I don't understand why. Any ideas?
> --
> kobi
>
>
> Program received signal:  "EXC_BAD_ACCESS".
> [Switching to process 17993 thread 0xb90f]
> (gdb) where
> #0  0x003a0d81 in resip::shared_count::~shared_count  
> (this=0x2e747273) at /Users/ss-kobi/source/client/mp2/ 
> resiprocate-1.2/rutil/SharedCount.hxx:247
> #1  0x003a0f34 in resip::SharedPtr<resip::SipMessage>::~SharedPtr  
> (this=0x2e74726f) at /Users/ss-kobi/source/client/mp2/ 
> resiprocate-1.2/rutil/SharedPtr.hxx:73
> #2  0x004212bd in resip::InviteSession::QueuedNIT::~QueuedNIT  
> (this=0x2e74726f) at ../resip/dum/InviteSession.hxx:352
> #3  0x001fae2a in resip::InviteSession::~InviteSession  
> (this=0xe352200) at /Users/ss-kobi/source/resiprocate-1.2.2/resip/ 
> dum/InviteSession.cxx:86
> #4  0x0044c091 in resip::ClientInviteSession::~ClientInviteSession  
> (this=0xe352200) at ../resip/dum/ClientInviteSession.hxx:13
> #5  0x002486d5 in resip::DestroyUsage::destroy (this=0xdcdf9d0) at / 
> Users/ss-kobi/source/resiprocate-1.2.2/resip/dum/DestroyUsage.cxx:87
> #6  0x001afcd5 in resip::DialogUsageManager::internalProcess  
> (this=0xe197200, msg=@0xb02d97e4) at /Users/ss-kobi/source/ 
> resiprocate-1.2.2/resip/dum/DialogUsageManager.cxx:1123
> #7  0x001b03f8 in resip::DialogUsageManager::process  
> (this=0xe197200, mutex=0x0) at /Users/ss-kobi/source/ 
> resiprocate-1.2.2/resip/dum/DialogUsageManager.cxx:1388
> #8  0x0011adf4 in SipEP::run (this=0x286f8cb0) at /Users/ss-kobi/ 
> source/client/xcode/../QvixApp/AppSrc/pstn/SipEP.cxx:3011
> #9  0x00114341 in SipEP::staticSDLThread (arg=0x286f8cb0) at /Users/ 
> ss-kobi/source/client/xcode/../QvixApp/AppSrc/pstn/SipEP.cxx:817
> #10 0x3002d649 in SDL_RunThread ()
> #11 0x3002f1e8 in RunThread ()
> #12 0x90024227 in _pthread_body ()
> Current language:  auto; currently c++
>
>
>
> On Feb 1, 2008, at 10:06 PM, Scott Godin wrote:
>
>
> When is an InviteSession ever copied?  I don’t think this is a  
> problem.
>
> From: Kobi Eshun [mailto:kobi at sightspeed.com]
> Sent: February-01-08 6:50 PM
> To: Scott Godin
> Cc: resiprocate-devel at resiprocate.org
> 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.
>
> --
> kobi
>
>
>
>
> 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 at resiprocate.org [mailto:resiprocate-
> > devel-bounces at resiprocate.org] On Behalf Of Kobi Eshun
> > Sent: Thursday, January 31, 2008 7:56 PM
> > To: resiprocate-devel at resiprocate.org
> > 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>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20080204/bfebde83/attachment.htm>

More information about the resiprocate-devel mailing list