[asterisk-dev] murf: branch 1.4 r152535 - in /branches/1.4: apps/ funcs/ include/asterisk/ res/

[Steve Murphy]

On Mon, 2008-11-03 at 15:28 -0600, Russell Bryant wrote:
> Mark Michelson wrote:
> >>> -       ast_set2_flag(c, autoloopflag, AST_FLAG_IN_AUTOLOOP);
> >>> -       ast_clear_flag(c, AST_FLAG_BRIDGE_HANGUP_RUN); /* from one round
> >>> to the next, make sure this gets cleared */
> >>> -       pbx_destroy(c->pbx);
> >>> -       c->pbx = NULL;
> >>> -       if (res != AST_PBX_KEEPALIVE)
> >>> +       if (res != AST_PBX_KEEPALIVE) {
> >>> +               ast_set2_flag(c, autoloopflag, AST_FLAG_IN_AUTOLOOP);
> >>> +               ast_clear_flag(c, AST_FLAG_BRIDGE_HANGUP_RUN); /* from
> >>> one round to the next, make sure this gets cleared */
> >>> +               pbx_destroy(c->pbx);
> >>> +               c->pbx = NULL;
> >>>                 ast_hangup(c);
> >>> +       }
> >>>         return 0;
> >>>  }
> >>>
> > 
> 
> > 
> >> You are correct that the existing code is not safe.  However, in the 
> >> existing code, you have the potential for a crash.  In your modified 
> >> version, you have a memory leak, as the ast_pbx isn't getting cleared up 
> >> like it is supposed to.
> >>
> > 
> > For my education, where is the memory leak? What else is required besides 
> > pbx_destroy()?
> 
> Previously, when code returned KEEPALIVE, it was expecting the PBX 
> thread to destroy the channel's ast_pbx object.  In this modified code, 
> pbx_destroy() is no longer being called.  The catch is, it's really not 
> safe to touch the channel at all here ...

Right, so the "right" thing to do is just make sure the parking manager
properly
destroys its channel, including destroying the pbx... then there would
be no
leak, right?

> 
-- 
Steve Murphy
Software Developer
Digium
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3227 bytes
Desc: not available
Url : http://lists.digium.com/pipermail/asterisk-dev/attachments/20081103/c9e078a7/attachment-0001.bin