< Previous by Date Date Index Next by Date >
< Previous in Thread Thread Index  

RE: [reSIProcate] AppDialogSet Memory Leak


I committed a change, the behaviour is now:
1.  If end is called then DialogSet is put in a WaitingToTerminate
state.
2.  If a 200 to subscribe is received in a WaitingToTerminate state,
then an un-subscribe is sent and the AppDialogSet is deleted.
3.  If a 1xx to subscribe is received in a WaitingToTerminate state,
then it is ignored.
4.  If another final response is received (ie. 408), the AppDialogSet is
deleted.

I have committed code for both client SUBSCRIBE and PUBLISH dialogs.
Perhaps we should also add similar code for client REGISTRATIONs and/or
Out-of-Dialog requests, etc.   Since these can also be "ended" via
AppDialogSet::end().

Scott

> -----Original Message-----
> From: resiprocate-devel-bounces@xxxxxxxxxxxxxxxxxxx
[mailto:resiprocate-
> devel-bounces@xxxxxxxxxxxxxxxxxxx] On Behalf Of Derek MacDonald
> Sent: Tuesday, March 14, 2006 1:54 PM
> To: 'Kevin Pickard'; resiprocate-devel@xxxxxxxxxxxxxxxxxxx
> Subject: RE: [reSIProcate] AppDialogSet Memory Leak
> 
> Hi Kevin,
> 
> There is no way to cancel a SUBSCRIBE request; the sip CANCEL method
only
> applies to invite.  To make AppDialogSet::end behave as you wish the
> ClientSubscription usage would have to be put into an "app no longer
> interested" state.  In this state, no callbacks would be called on the
> handler, and if the subscription *succeeds* the ClientSubscription
usage
> would immediately unsubscribe.  The AppDialogSet could be released
when
> end() is called *if* there are no other dialogs or usages in that
> dialogset.
> 
> While doing this for ClientSubscription would be relatively
> straightforward,
> there is some complexity w/ multiple usages in a dialog, and multiple
> dialogs in a DialogSet when determining AppDialogSet lifetime that
would
> have to be worked through.
> 
> I think this is worth adding as AppDialogSet::end currently does not
have
> the correct semantics for SUBSCRIBE dialogs, so it would be great if
you
> want to take a crack at it.
> 
> --Derek
> 
> 
> 
> -----Original Message-----
> From: resiprocate-devel-bounces@xxxxxxxxxxxxxxxxxxx
> [mailto:resiprocate-devel-bounces@xxxxxxxxxxxxxxxxxxx] On Behalf Of
Kevin
> Pickard
> Sent: Tuesday, March 14, 2006 5:52 AM
> To: resiprocate-devel@xxxxxxxxxxxxxxxxxxx
> Subject: Fwd: [reSIProcate] AppDialogSet Memory Leak
> 
>          Hello everyone. I posted this a week ago but no one has
> responded.
> Before I go and write what I think is missing code, can someone let me
> know
> if I am even on the right track?
> Thanks.
> 
> >Date: Wed, 08 Mar 2006 12:06:11 -0500
> >To: <resiprocate-devel@xxxxxxxxxxxxxxxxxxx>
> >
> >         Hello everyone.
> >
> >         We have encountered a problem that is resulting in a memory
leak
> > due to AppDialogSets that are no longer being used but never getting
> > released/destroyed. I have taken a look at the DUM source and have
> > identified the reason it is happening. It looks like the code for
> > handling our particular scenario has not been completed. So I am
> > wondering if what we are doing is correct and the code has just not
been
> > written yet or if there is a better way of handling this situation.
> >
> >         Basically the scenario is that we create a SUBSCRIBE request
via
> > Dum.makeSubscription() passing in our own AppDialogSet. We then send
the
> > request. Now the device we are subscribing to never responds (for
> > whatever reason) and the request eventually times out with an
internally
> > generated 408 SipResp. In this simple situation DUM eventually calls
> > destroy() on the AppDialogSet and things get cleaned up nicely.
> >
> >         The problem occurs if we try to kill the request *before*
the
> > timeout occurs. We are currently calling end() on the AppDialogSet
to do
> > this. This results in the state of the underlying DialogSet being
set to
> > WaitingToEnd. Now when the 408 timeout response is then generated,
it
> > eventually gets passed through to DialogSet::dispatch() for
processing.
> > As can be seen from the code shown below, using a Release build, the
> > resulting action is to do nothing. With a debug build it will
assert().
> > In our scenario mState is WaitingToEnd and the last method was
> SUBSCRIBE.
> >
> >         So are we doing the right thing? Is it just a matter that no
one
> > has written the handling code for SUBSCRIBE yet? The code for
handling
> > the same situation for INVITE is present and I would expect that the
> > equivalent code for handling the SUBSCRIBE situation would be
similar.
> > Otherwise, is there a better way of killing the pending SUBSCRIBE?
> >
> >         Thanks.
> >
> >
> >void
> >DialogSet::dispatch(const SipMessage& msg)
> >{
> >    assert(msg.isRequest() || msg.isResponse());
> >
> >    if (mState == WaitingToEnd)
> >    {
> >       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)
> >                {
> >                   mState = ReceivedProvisional;
> >                   end();
> >                }
> >                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;
> >             case SUBSCRIBE:
> >                assert(0);
> >                break;
> >             default:
> >                break;
> >          }
> >       }
> 
> 
> 
> _______________________________________________
> resiprocate-devel mailing list
> resiprocate-devel@xxxxxxxxxxxxxxxxxxx
> https://list.sipfoundry.org/mailman/listinfo/resiprocate-devel
> 
> _______________________________________________
> resiprocate-devel mailing list
> resiprocate-devel@xxxxxxxxxxxxxxxxxxx
> https://list.sipfoundry.org/mailman/listinfo/resiprocate-devel