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

[Russell Bryant]

Steve Murphy wrote:

> The goal was to properly set up channel CDR's in a transfer situation,
> so that the next leg had the proper start times; but it occurs to
> me now, that it might be possible to move this code (well, at least
> the job it wishes to do) from after the  bridge, to the transfer code 
> itself. I will investigate this and  report back in this forum.
> Hopefully
> in the next few days. If I can replicate previous behavior, then this
> should make clean up the after-bridge code considerably.

Ok, this sounds like it could avoid the problems.  We'll see what you 
come up with.  :)

> By the way, I found some parking related code in main/pbx.c that I would
> assume
> needs to be changed to protect against illegal access after the bridge
> returns AST_PBX_KEEPALIVE in __ast_pbx_run()...
> 
> -       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;
>  }
> 
> The interesting thing is that the present version didn't have problems
> in the
> various "tired" scenarios, or at least, didn't seem to to; but the
> changes 
> proposed above seem like a good idea to me. Am I correct?

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.

Honestly, this KEEPALIVE stuff just needs to go away completely.  Trying 
to hack the code in various places to try to make it safe is going to be 
an uphill battle, I'm afraid.

-- 
Russell Bryant
Senior Software Engineer
Open Source Team Lead
Digium, Inc.