[reSIProcate] NULL pointer crash in DialogSet::dispatch withmAppDialogSet

Byron Campen bcampen at estacado.net
Fri Oct 5 16:37:30 CDT 2007


	Agreed. I would like to see it go away entirely.

Best regards,
Byron Campen

> Sorry for the late response….
>
>
>
> I think the fix you made is a good bandaid for now.  You are right  
> we have a real issue with AppDialogSet::reuse() and DialogSets that  
> have multiple Dialogs.  We should look at a more permanent solution  
> in a future release.  Currently Reuse() is only used for  
> ClientSubscriptions – I think we should revisit if it is even  
> required.
>
>
>
> Scott
>
>
>
> From: Byron Campen [mailto:bcampen at estacado.net]
> Sent: Wednesday, September 26, 2007 5:04 PM
> To: Scott Godin
> Cc: Aron Rosenberg; resiprocate-devel at list.resiprocate.org
> Subject: Re: [reSIProcate] NULL pointer crash in  
> DialogSet::dispatch withmAppDialogSet
>
>
>
>           So, I've given this a look, and I have a theory. Aron  
> said that the crash happened sometime after the onRequestRetry()  
> callback returned 0...
>
>
>
> From ClientSubscription.cxx, starting at line 165
>
> *snip*
>
>       if (msg.header(h_StatusLine).statusCode() == 408)
>
>       {
>
>          InfoLog (<< "Received 408 to SUBSCRIBE "
>
>                   << mLastRequest->header(h_To));
>
>          retry = handler->onRequestRetry(getHandle(), 0, msg);
>
>       }
>
>       else
>
>       {
>
>          InfoLog (<< "Received non-408 retriable to SUBSCRIBE "
>
>                   << mLastRequest->header(h_To));
>
>          retry = handler->onRequestRetry(getHandle(), msg.header 
> (h_RetryAfter).value(), msg);
>
>       }
>
>
>
>       if (retry < 0)
>
>       {
>
>          DebugLog(<< "Application requested failure on Retry-After");
>
>          mEnded = true;
>
>          handler->onTerminated(getHandle(), msg);
>
>          delete this;
>
>          return;
>
>       }
>
>       else if (retry == 0)
>
>       {
>
>          DebugLog(<< "Application requested immediate retry on  
> Retry-After");
>
>          //!dcm! -- why isn't this just a refresh--retry after  
> might be
>
>          //middle element and not indicate dialog destruction
>
>          if (mDialog.mRemoteTarget.uri().host().empty())
>
>          {
>
>             SharedPtr<SipMessage> sub = mDum.makeSubscription 
> (mLastRequest->header(h_To), getEventType());
>
>             mDum.send(sub);
>
>          }
>
>          else
>
>          {
>
>             SharedPtr<SipMessage> sub = mDum.makeSubscription 
> (mDialog.mRemoteTarget, getEventType(), getAppDialogSet()->reuse());
>
>             mDum.send(sub);
>
>          }
>
>          //!dcm! -- new sub created above, when does this usage get  
> destroyed?
>
>          //return;
>
>       }
>
>       else
>
>       {
>
>          // leave the usage around until the timeout
>
>          // !dlb! would be nice to set the state to something dead,  
> but not used
>
>          mDum.addTimer(DumTimeout::SubscriptionRetry,
>
>                        retry,
>
>                        getBaseHandle(),
>
>                        ++mTimerSeq);
>
>          // leave the usage around until the timeout
>
>          return;
>
>       }
>
>
>
>       delete this;
>
>       return;
>
>
>
> *snip*
>
>
>
>     So, returning 0 on the onRequestRetry() callback causes the  
> AppDialogSet* to be unset (See impl of AppDialogSet::reuse()), and  
> the ClientSubscription to be deleted. Then, in ~DialogUsage() (from  
> which ClientSubscription is derived)...
>
>
>
> *snip*
>
> DialogUsage::~DialogUsage()
>
> {
>
>    mDialog.possiblyDie();
>
> }
>
> *snip*
>
>
>
> When the Dialog gets this call, it calls DialogUsageManager::destroy 
> () on itself, like so:
>
>
>
> *snip*
>
> void Dialog::possiblyDie()
>
> {
>
>    // !slg! Note:  dialogs should really stick around for 32s, in  
> order to ensure that all 2xx retransmissions get Ack'd, then BYE'd  
> correctly
>
>    if (!mDestroying)
>
>    {
>
>       if (mClientSubscriptions.empty() &&
>
>           mServerSubscriptions.empty() &&
>
>           !mInviteSession)
>
>       {
>
>          mDestroying = true;
>
>          mDum.destroy(this);
>
>       }
>
>    }
>
> }
>
> *snip*
>
>
>
> Which in turn causes the following to happen:
>
>
>
> *snip*
>
> void
>
> DialogUsageManager::destroy(Dialog* d)
>
> {
>
>    if (mShutdownState != Destroying)
>
>    {
>
>       post(new DestroyUsage(d));
>
>    }
>
>    else
>
>    {
>
>       InfoLog(<< "DialogUsageManager::destroy() not posting to  
> stack");
>
>    }
>
> }
>
> *snip*
>
>
>
>     So, we asynchronously schedule the destruction of the Dialog.  
> The DialogSet is still around. If some messaging comes in, and  
> beats the DestroyUsage message to the front of the queue (not an  
> unlikely scenario at all, and made much more likely by sitting at a  
> breakpoint), the messaging will be sent to the DialogSet, which  
> will then core in exactly the manner you're seeing.
>
>
>
>     Ultimately, if the AppDialogSet is reused, we have to be  
> completely sure that the DialogSet will be gone before any further  
> messaging can arrive.
>
>
>
>     On top of this, we have the issue of what happens when we have  
> more than one Dialog in this DialogSet (say, the SUBSCRIBE got  
> forked). If one of the forks decides it needs to start a new  
> subscription in the manner above, the other subscriptions in the  
> DialogSet will find themselves in trouble very shortly. This isn't  
> even a race. It's basically guaranteed to fail.
>
>
>
>     My inclination is to turn AppDialogSet::reuse() into a clone(),  
> or into a call to the AppDialogSetFactory. (Or, we need to just be  
> tolerant if the AppDialogSet is null. But I imagine we have opened  
> a rather large can or worms here, and there are probably other  
> nasty race-conditions that we'll encounter as a result of the non- 
> atomicity of DialogSet teardown.)
>
>
>
> Best regards,
>
> Byron Campen
>
>
>
>
> It’s possible you’ve exposed a race condition by using the debugger  
> – I can’t think of what it would be offhand though. : (  The Debug  
> log should provide more insight.
>
>
>
> Scott
>
>
>
> From: Aron Rosenberg [mailto:arosenberg at sightspeed.com]
> Sent: Friday, July 27, 2007 2:48 PM
> To: Scott Godin; resiprocate-devel at list.resiprocate.org
> Subject: RE: [reSIProcate] NULL pointer crash in  
> DialogSet::dispatch withmAppDialogSet
>
>
>
> One thing that might be going on is that when I sit in a  
> breakpoint, a 408 transaction is created by the resip stack because  
> of the “timeout”. Could this because the reason for the crash,  
> onRemoved gets called, but a 408 is still active…
>
>
>
> -Aron
>
>
>
> From: Scott Godin [mailto:slgodin at icescape.com]
> Sent: Friday, July 27, 2007 10:24 AM
> To: Aron Rosenberg; resiprocate-devel at list.resiprocate.org
> Subject: RE: [reSIProcate] NULL pointer crash in  
> DialogSet::dispatch withmAppDialogSet
>
>
>
> Are you calling all DUM API’s from the same thread (including the  
> process loop)?
>
>
>
> From: Aron Rosenberg [mailto:arosenberg at sightspeed.com]
> Sent: Friday, July 27, 2007 12:39 PM
> To: Scott Godin; resiprocate-devel at list.resiprocate.org
> Subject: RE: [reSIProcate] NULL pointer crash in  
> DialogSet::dispatch withmAppDialogSet
>
>
>
> DUM controls everything. This crash only happens if I have hit a  
> breakpoint and let it sit for a couple seconds before continuing.  
> For example, If I have a breakpoint in the onTerminated or  
> onRequestRetry for ClientSubscriptionHandler implementation. It  
> will crash after the continue. This never happens except in debug  
> mode and only if breakpoints were used on the resiprocate thread.
>
>
>
> -Aron
>
>
>
> From: Scott Godin [mailto:slgodin at icescape.com]
> Sent: Friday, July 27, 2007 6:48 AM
> To: Aron Rosenberg; resiprocate-devel at list.resiprocate.org
> Subject: RE: [reSIProcate] NULL pointer crash in  
> DialogSet::dispatch withmAppDialogSet
>
>
>
> If the DialogSet is still around the AppDialogSet should also be  
> around.  Are you deleting your AppDialogSet instances or letting  
> DUM control their lifetime?
>
>
>
> Scott
>
>
>
> From: resiprocate-devel-bounces at list.resiprocate.org  
> [mailto:resiprocate-devel-bounces at list.resiprocate.org] On Behalf  
> Of Aron Rosenberg
> Sent: Thursday, July 26, 2007 12:20 PM
> To: resiprocate-devel at list.resiprocate.org
> Subject: [reSIProcate] NULL pointer crash in DialogSet::dispatch  
> withmAppDialogSet
>
>
>
> So I have been debugging some resip issues with ClientSubscriptions  
> and keep getting a NULL pointer crash after I have been sitting at  
> some breakpoints in the code. The crash occurs in:
>
>
>
> DialogSet.cxx line 775
>
>       DebugLog ( << "### Calling CreateAppDialog ###: " <<  
> std::endl << std::endl <<msg);
>
> --->  AppDialog* appDialog = mAppDialogSet->createAppDialog(msg);
>
>       dialog->mAppDialog = appDialog;
>
>       appDialog->mDialog = dialog;
>
>       dialog->dispatch(msg);
>
>
>
> mAppDialogSet is NULL. The call stack for this crash is as follows:
>
>
>
> >          resip::DialogSet::dispatch(const resip::SipMessage & msg= 
> {...})  Line 775 + 0x1c  C++
>
>             resip::DialogUsageManager::processRequest(const  
> resip::SipMessage & request={...})  Line 1560   C++
>
>             resip::DialogUsageManager::incomingProcess 
> (std::auto_ptr<resip::Message> msg={...})  Line 1281 C++
>
>             resip::DialogUsageManager::internalProcess 
> (std::auto_ptr<resip::Message> msg={...})  Line 1134   C++
>
>             resip::DialogUsageManager::process()  Line 1300 + 0x49   
> C++
>
>             SipEP::run()  Line 2404 + 0xb     C++ // Is our thread  
> loop
>
>
>
>
>
> mState == Established
>
> msg is a NOTIFY
>
>
>
> The crash happens after I have requested the ->end() of the  
> ClientSubscription and a NOTIFY comes in from the wire and I call  
> rejectUpdate(404) since the user has “ended” the Client  
> Subscription. Then I usually get a call to onRequestRetry where we  
> return 0. The crash generally occurs after that. I will try to get  
> a resip debug log up to the crash.
>
>
>
> Does this appear to be a resip issue or are we improperly  
> implementing our own AppDialog and AppDialogSet instances.
>
>
>
> -Aron
>
>
>
>
>
> ---------------------------------------------
>
> Aron Rosenberg
>
> Founder and CTO
>
> SightSpeed - http://www.sightspeed.com/
>
>
>
> 918 Parker St, Suite A14
>
> Berkeley, CA 94710
>
>
>
> Email: arosenberg at sightspeed.com
>
> Phone: 510-665-2920
>
> Cell: 510-847-7389
>
> Fax: 510-649-9569
>
> SightSpeed Video Link: http://aron.sightspeed.com
>
>
>
>
>
>
>
> _______________________________________________
>
> resiprocate-devel mailing list
>
> resiprocate-devel at list.resiprocate.org
>
> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>
>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20071005/db46545e/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 2423 bytes
Desc: not available
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20071005/db46545e/attachment.bin>


More information about the resiprocate-devel mailing list