[asterisk-dev] [Code Review]: Restore default_user and default_bridge profiles in ConfBridge when their sections are removed from the config file
Matt Jordan
reviewboard at asterisk.org
Sat Feb 23 19:30:39 CST 2013
> On Feb. 23, 2013, 4:38 p.m., Joshua Colp wrote:
> > There is a race condition here between applying the config and creating these default profiles. You should use the pre apply callback in the config framework so these exist before being applied.
>
> Matt Jordan wrote:
> Yup, between apply_config and the verifying of the default profiles, someone could attempt to use Page and not get a default profile. Nuts.
>
> I'm not sure the pre-apply callback will work. If the pre-apply callback passed you the aco_info object it would work, but unfortunately during the pre-apply callback, you have access to the stale data in the global container, not the pending data in the aco_info object. When apply_config is called, it will blow out the global container - so any profiles you've stuck in there will be lost.
>
> The post_apply_config should work, however, as the global container will contain the default profiles. Changed to use that instead.
Bah, that actually doesn't work either. There's still a race condition, as the global container already swapped out the contents.
The pre-apply config really does need to pass you the pending ao2 container that it's about to swap into the global container so you can safely modify it. That'd be a change to the header file in a release branch however.
We could either change the API in 11, or suffer with the small race condition and fix it in trunk.
- Matt
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2356/#review7923
-----------------------------------------------------------
On Feb. 23, 2013, 7:28 p.m., Matt Jordan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2356/
> -----------------------------------------------------------
>
> (Updated Feb. 23, 2013, 7:28 p.m.)
>
>
> Review request for Asterisk Developers.
>
>
> Summary
> -------
>
> The confbridge sample configuration file has the following to say about the default user/bridge profiles:
>
> ; --- Default Information ---
> ; The default_user and default_bridge sections are applied
> ; automatically to all ConfBridge instances invoked without
> ; a user, or bridge argument. No menu is applied by default.
> ;
>
> These always have to exist: while they can be specified in the conf file and their default values overriden, removing them from the conf file should not remove them from ConfBridge - it has to have a default profile to apply to bridges/users, otherwise it doesn't know what to do with them when you fail to specify a profile. What's more, applications such as Page (which use ConfBridge under the hood) have no mechanism to supply a bridge profile to build on, resulting in errors.
>
> This patch restores the behavior prior to the configuration re-work that went in for ConfBridge for Asterisk 11. It ensures that if the conf file processed does not provide a default bridge/user profile, that the objects are created and populated with their default values appropriately.
>
>
> Diffs
> -----
>
> /branches/11/apps/confbridge/conf_config_parser.c 381915
>
> Diff: https://reviewboard.asterisk.org/r/2356/diff
>
>
> Testing
> -------
>
> Removed the default_bridge/default_user profiles from the configuration file. Started Asterisk, CLI command successfully showed that the default profiles existed in memory with the appropriate values.
>
>
> Thanks,
>
> Matt
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130224/170011c4/attachment.htm>
More information about the asterisk-dev
mailing list