Re: [asterisk-dev] [Code Review] 2961: Add channel locking for calls that result in channel snapshot creation

2013-11-25 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2961/#review10273
---



/branches/12/main/bridge.c


blob




/branches/12/main/core_local.c


Revert.  The source is already locked as one of the seven locks held.



/branches/12/main/core_local.c


Revert all the changes you've made to local_call() and reacquire the locks 
in publish_local_bridge_message() using ast_unreal_lock_all().  As it is you 
have paths that leave channels locked.



/branches/12/main/pickup.c


Simplify the locking like this:

ast_channel_lock(chan);
chan_snapshot = ast_channel_snapshot_create(chan);
ast_channel_unlock(chan);
if (!chan_snapshot) {
goto pickup_failed;
}

ast_channel_lock(target);
target_snapshot = ast_channel_snapshot_create(target);
ast_channel_unlock(target);
if (!target_snapshot) {
goto pickup_failed;
}



- rmudgett


On Nov. 19, 2013, 11:17 p.m., Mark Michelson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2961/
> ---
> 
> (Updated Nov. 19, 2013, 11:17 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22709
> https://issues.asterisk.org/jira/browse/ASTERISK-22709
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> John Bigelow discovered that if the newly-created atxfer_threeway_nominal 
> test is run in a loop, eventually a test run will cause the UUT instance of 
> Asterisk to crash. Investigation of the crash showed that the crash was being 
> caused while trying to get the string representation of a channel's 
> translation paths while creating a channel snapshot. The attended transfer 
> manager thread was creating the channel snapshot as part of the attended 
> transfer stasis message while in another thread (in this case, the bridge 
> channel thread) the translation paths were being altered to make the channel 
> compatible with others in the bridge.
> 
> It's clear that the issue is that the channel's contents are not lock 
> protected while creating a channel snapshot. Luckily, the channel's lock is 
> held already when updating translation paths. By adding locking, I was able 
> to run the test over 450 times straight with no negative consequences.
> 
> When adding locking, there were two strategies to follow:
> 
> 1) Take advantage of the fact that channel locks are recursive, and just add 
> a single SCOPED_CHANNELLOCK() call to ast_channel_snapshot_create() and call 
> it a day.
> 2) Make an audit of channel-related calls that end up creating a channel 
> snapshot and require locks be held prior to making those calls.
> 
> I like approach 2 more, even though it's harder to pull off. In investigating 
> what paths result in channel snapshots being created, I found that channel 
> locks should be required before calling the following functions:
> 
> * ast_channel_snapshot_create()
> * ast_channel_blob_create()
> * ast_channel_publish_snapshot()
> * ast_publish_channel_state()
> * ast_channel_publish_blob()
> * ast_channel_publish_varset()
> * ast_channel_amaflags_set()
> * ast_channel_callid_set()
> * ast_channel_whentohangup_set()
> * ast_channel_callgroup_set()
> * ast_channel_pickupgroup_set()
> * ast_channel_internal_bridge_set()
> * ast_channel_language_set()
> * ast_channel_accountcode_set()
> * ast_channel_peeraccount_set()
> * ast_channel_linkedid_set()
> * ast_channel_stage_snapshot()
> * ast_setstate()
> * ast_aoc_manager_event()
> * ast_channel_setwhentohangup_tv()
> * ast_channel_setwhentohangup()
> * ast_set_variables()
> 
> All of these functions' documentation have been updated to indicate a 
> precondition of having the channel locked.
> 
> In addition, there are some more complex functions that create channel 
> snapshots but could not easily be made to have the precondition of having the 
> channel locked. These are:
> 
> * ast_bridge_blob_create()
> * ast_publish_attended_transfer_fail()
> * ast_publish_attended_transfer_bridge_merge()
> * ast_publish_attended_transfer_threeway()
> * ast_publish_attended_transfer_app()
> * ast_publish_attended_transfer_link()
> * ast_bridge_publish_enter()
> * ast_bridge_publish_leave()
> * ast_bridge_publish_blind_transfer()
> 
> These functions' documentation now have a precondition of having no channels 
> or bridges locked.
> 
> This review seeks only to make sure that channels are properly locked prior 
> to creating channel snapshots. A similar effort is lik

Re: [asterisk-dev] [Code Review] 2961: Add channel locking for calls that result in channel snapshot creation

2013-11-25 Thread Mark Michelson 


> On Nov. 25, 2013, 6:54 p.m., rmudgett wrote:
> > /branches/12/main/core_local.c, lines 682-685
> > 
> >
> > Revert all the changes you've made to local_call() and reacquire the 
> > locks in publish_local_bridge_message() using ast_unreal_lock_all().  As it 
> > is you have paths that leave channels locked.

I see one path that leaves the channels locked. Why not just correct that one 
path? It seems wasteful to reacquire the locks this way when the parent 
function that calls publish_local_bridge_message() has already done the work to 
acquire the locks once.


- Mark


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2961/#review10273
---


On Nov. 19, 2013, 11:17 p.m., Mark Michelson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2961/
> ---
> 
> (Updated Nov. 19, 2013, 11:17 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22709
> https://issues.asterisk.org/jira/browse/ASTERISK-22709
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> John Bigelow discovered that if the newly-created atxfer_threeway_nominal 
> test is run in a loop, eventually a test run will cause the UUT instance of 
> Asterisk to crash. Investigation of the crash showed that the crash was being 
> caused while trying to get the string representation of a channel's 
> translation paths while creating a channel snapshot. The attended transfer 
> manager thread was creating the channel snapshot as part of the attended 
> transfer stasis message while in another thread (in this case, the bridge 
> channel thread) the translation paths were being altered to make the channel 
> compatible with others in the bridge.
> 
> It's clear that the issue is that the channel's contents are not lock 
> protected while creating a channel snapshot. Luckily, the channel's lock is 
> held already when updating translation paths. By adding locking, I was able 
> to run the test over 450 times straight with no negative consequences.
> 
> When adding locking, there were two strategies to follow:
> 
> 1) Take advantage of the fact that channel locks are recursive, and just add 
> a single SCOPED_CHANNELLOCK() call to ast_channel_snapshot_create() and call 
> it a day.
> 2) Make an audit of channel-related calls that end up creating a channel 
> snapshot and require locks be held prior to making those calls.
> 
> I like approach 2 more, even though it's harder to pull off. In investigating 
> what paths result in channel snapshots being created, I found that channel 
> locks should be required before calling the following functions:
> 
> * ast_channel_snapshot_create()
> * ast_channel_blob_create()
> * ast_channel_publish_snapshot()
> * ast_publish_channel_state()
> * ast_channel_publish_blob()
> * ast_channel_publish_varset()
> * ast_channel_amaflags_set()
> * ast_channel_callid_set()
> * ast_channel_whentohangup_set()
> * ast_channel_callgroup_set()
> * ast_channel_pickupgroup_set()
> * ast_channel_internal_bridge_set()
> * ast_channel_language_set()
> * ast_channel_accountcode_set()
> * ast_channel_peeraccount_set()
> * ast_channel_linkedid_set()
> * ast_channel_stage_snapshot()
> * ast_setstate()
> * ast_aoc_manager_event()
> * ast_channel_setwhentohangup_tv()
> * ast_channel_setwhentohangup()
> * ast_set_variables()
> 
> All of these functions' documentation have been updated to indicate a 
> precondition of having the channel locked.
> 
> In addition, there are some more complex functions that create channel 
> snapshots but could not easily be made to have the precondition of having the 
> channel locked. These are:
> 
> * ast_bridge_blob_create()
> * ast_publish_attended_transfer_fail()
> * ast_publish_attended_transfer_bridge_merge()
> * ast_publish_attended_transfer_threeway()
> * ast_publish_attended_transfer_app()
> * ast_publish_attended_transfer_link()
> * ast_bridge_publish_enter()
> * ast_bridge_publish_leave()
> * ast_bridge_publish_blind_transfer()
> 
> These functions' documentation now have a precondition of having no channels 
> or bridges locked.
> 
> This review seeks only to make sure that channels are properly locked prior 
> to creating channel snapshots. A similar effort is likely needed to ensure 
> that bridges are locked during bridge snapshot creation.
> 
> 
> Diffs
> -
> 
>   /branches/12/tests/test_stasis_channels.c 402818 
>   /branches/12/tests/test_cel.c 402818 
>   /branches/12/tests/test_cdr.c 402818 
>   /branches/12/res/res_stasis.c 402818 
>   /branches/12/res/res_pjsip_refer.c 402818 
>   /branches/12/res/res_agi.c 402818 
>   /branches/12/res/parkin

Re: [asterisk-dev] [Code Review] 2961: Add channel locking for calls that result in channel snapshot creation

2013-11-25 Thread rmudgett 


> On Nov. 25, 2013, 6:54 p.m., rmudgett wrote:
> > /branches/12/main/core_local.c, lines 682-685
> > 
> >
> > Revert all the changes you've made to local_call() and reacquire the 
> > locks in publish_local_bridge_message() using ast_unreal_lock_all().  As it 
> > is you have paths that leave channels locked.
> 
> Mark Michelson wrote:
> I see one path that leaves the channels locked. Why not just correct that 
> one path? It seems wasteful to reacquire the locks this way when the parent 
> function that calls publish_local_bridge_message() has already done the work 
> to acquire the locks once.

It is simpler to verify that the locks are correct if 
publish_local_bridge_message() does its own locking.


- rmudgett


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2961/#review10273
---


On Nov. 19, 2013, 11:17 p.m., Mark Michelson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2961/
> ---
> 
> (Updated Nov. 19, 2013, 11:17 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22709
> https://issues.asterisk.org/jira/browse/ASTERISK-22709
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> John Bigelow discovered that if the newly-created atxfer_threeway_nominal 
> test is run in a loop, eventually a test run will cause the UUT instance of 
> Asterisk to crash. Investigation of the crash showed that the crash was being 
> caused while trying to get the string representation of a channel's 
> translation paths while creating a channel snapshot. The attended transfer 
> manager thread was creating the channel snapshot as part of the attended 
> transfer stasis message while in another thread (in this case, the bridge 
> channel thread) the translation paths were being altered to make the channel 
> compatible with others in the bridge.
> 
> It's clear that the issue is that the channel's contents are not lock 
> protected while creating a channel snapshot. Luckily, the channel's lock is 
> held already when updating translation paths. By adding locking, I was able 
> to run the test over 450 times straight with no negative consequences.
> 
> When adding locking, there were two strategies to follow:
> 
> 1) Take advantage of the fact that channel locks are recursive, and just add 
> a single SCOPED_CHANNELLOCK() call to ast_channel_snapshot_create() and call 
> it a day.
> 2) Make an audit of channel-related calls that end up creating a channel 
> snapshot and require locks be held prior to making those calls.
> 
> I like approach 2 more, even though it's harder to pull off. In investigating 
> what paths result in channel snapshots being created, I found that channel 
> locks should be required before calling the following functions:
> 
> * ast_channel_snapshot_create()
> * ast_channel_blob_create()
> * ast_channel_publish_snapshot()
> * ast_publish_channel_state()
> * ast_channel_publish_blob()
> * ast_channel_publish_varset()
> * ast_channel_amaflags_set()
> * ast_channel_callid_set()
> * ast_channel_whentohangup_set()
> * ast_channel_callgroup_set()
> * ast_channel_pickupgroup_set()
> * ast_channel_internal_bridge_set()
> * ast_channel_language_set()
> * ast_channel_accountcode_set()
> * ast_channel_peeraccount_set()
> * ast_channel_linkedid_set()
> * ast_channel_stage_snapshot()
> * ast_setstate()
> * ast_aoc_manager_event()
> * ast_channel_setwhentohangup_tv()
> * ast_channel_setwhentohangup()
> * ast_set_variables()
> 
> All of these functions' documentation have been updated to indicate a 
> precondition of having the channel locked.
> 
> In addition, there are some more complex functions that create channel 
> snapshots but could not easily be made to have the precondition of having the 
> channel locked. These are:
> 
> * ast_bridge_blob_create()
> * ast_publish_attended_transfer_fail()
> * ast_publish_attended_transfer_bridge_merge()
> * ast_publish_attended_transfer_threeway()
> * ast_publish_attended_transfer_app()
> * ast_publish_attended_transfer_link()
> * ast_bridge_publish_enter()
> * ast_bridge_publish_leave()
> * ast_bridge_publish_blind_transfer()
> 
> These functions' documentation now have a precondition of having no channels 
> or bridges locked.
> 
> This review seeks only to make sure that channels are properly locked prior 
> to creating channel snapshots. A similar effort is likely needed to ensure 
> that bridges are locked during bridge snapshot creation.
> 
> 
> Diffs
> -
> 
>   /branches/12/tests/test_stasis_channels.c 402818 
>   /branches/12/tests/test_cel.c 402818 
>   /branches/12/tests/test_cdr.c 402818 
>

Re: [asterisk-dev] [Code Review] 2961: Add channel locking for calls that result in channel snapshot creation

2013-11-26 Thread Mark Michelson 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2961/
---

(Updated Nov. 26, 2013, 6:13 p.m.)


Review request for Asterisk Developers.


Changes
---

Did the things that were suggested.


Bugs: ASTERISK-22709
https://issues.asterisk.org/jira/browse/ASTERISK-22709


Repository: Asterisk


Description
---

John Bigelow discovered that if the newly-created atxfer_threeway_nominal test 
is run in a loop, eventually a test run will cause the UUT instance of Asterisk 
to crash. Investigation of the crash showed that the crash was being caused 
while trying to get the string representation of a channel's translation paths 
while creating a channel snapshot. The attended transfer manager thread was 
creating the channel snapshot as part of the attended transfer stasis message 
while in another thread (in this case, the bridge channel thread) the 
translation paths were being altered to make the channel compatible with others 
in the bridge.

It's clear that the issue is that the channel's contents are not lock protected 
while creating a channel snapshot. Luckily, the channel's lock is held already 
when updating translation paths. By adding locking, I was able to run the test 
over 450 times straight with no negative consequences.

When adding locking, there were two strategies to follow:

1) Take advantage of the fact that channel locks are recursive, and just add a 
single SCOPED_CHANNELLOCK() call to ast_channel_snapshot_create() and call it a 
day.
2) Make an audit of channel-related calls that end up creating a channel 
snapshot and require locks be held prior to making those calls.

I like approach 2 more, even though it's harder to pull off. In investigating 
what paths result in channel snapshots being created, I found that channel 
locks should be required before calling the following functions:

* ast_channel_snapshot_create()
* ast_channel_blob_create()
* ast_channel_publish_snapshot()
* ast_publish_channel_state()
* ast_channel_publish_blob()
* ast_channel_publish_varset()
* ast_channel_amaflags_set()
* ast_channel_callid_set()
* ast_channel_whentohangup_set()
* ast_channel_callgroup_set()
* ast_channel_pickupgroup_set()
* ast_channel_internal_bridge_set()
* ast_channel_language_set()
* ast_channel_accountcode_set()
* ast_channel_peeraccount_set()
* ast_channel_linkedid_set()
* ast_channel_stage_snapshot()
* ast_setstate()
* ast_aoc_manager_event()
* ast_channel_setwhentohangup_tv()
* ast_channel_setwhentohangup()
* ast_set_variables()

All of these functions' documentation have been updated to indicate a 
precondition of having the channel locked.

In addition, there are some more complex functions that create channel 
snapshots but could not easily be made to have the precondition of having the 
channel locked. These are:

* ast_bridge_blob_create()
* ast_publish_attended_transfer_fail()
* ast_publish_attended_transfer_bridge_merge()
* ast_publish_attended_transfer_threeway()
* ast_publish_attended_transfer_app()
* ast_publish_attended_transfer_link()
* ast_bridge_publish_enter()
* ast_bridge_publish_leave()
* ast_bridge_publish_blind_transfer()

These functions' documentation now have a precondition of having no channels or 
bridges locked.

This review seeks only to make sure that channels are properly locked prior to 
creating channel snapshots. A similar effort is likely needed to ensure that 
bridges are locked during bridge snapshot creation.


Diffs (updated)
-

  /branches/12/tests/test_stasis_channels.c 402818 
  /branches/12/tests/test_cel.c 402818 
  /branches/12/tests/test_cdr.c 402818 
  /branches/12/res/res_stasis.c 402818 
  /branches/12/res/res_pjsip_refer.c 402818 
  /branches/12/res/res_agi.c 402818 
  /branches/12/res/parking/parking_manager.c 402818 
  /branches/12/res/parking/parking_bridge_features.c 402818 
  /branches/12/pbx/pbx_realtime.c 402818 
  /branches/12/main/stasis_channels.c 402818 
  /branches/12/main/stasis_bridges.c 402818 
  /branches/12/main/pickup.c 402818 
  /branches/12/main/pbx.c 402818 
  /branches/12/main/endpoints.c 402818 
  /branches/12/main/dial.c 402818 
  /branches/12/main/core_unreal.c 402818 
  /branches/12/main/core_local.c 402818 
  /branches/12/main/channel.c 402818 
  /branches/12/main/cel.c 402818 
  /branches/12/main/bridge_channel.c 402818 
  /branches/12/main/bridge.c 402818 
  /branches/12/include/asterisk/stasis_channels.h 402818 
  /branches/12/include/asterisk/stasis_bridges.h 402818 
  /branches/12/include/asterisk/channelstate.h 402818 
  /branches/12/include/asterisk/channel.h 402818 
  /branches/12/include/asterisk/aoc.h 402818 
  /branches/12/funcs/func_timeout.c 402818 
  /branches/12/channels/sig_pri.c 402818 
  /branches/12/channels/sig_analog.c 402818 
  /branches/12/channels/chan_vpb.cc 402818 
  /branches/12/channels/chan_unistim.c 402818 
  /bran

Re: [asterisk-dev] [Code Review] 2961: Add channel locking for calls that result in channel snapshot creation

2013-11-26 Thread rmudgett 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2961/#review10285
---

Ship it!


Ship It!

- rmudgett


On Nov. 26, 2013, 6:13 p.m., Mark Michelson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2961/
> ---
> 
> (Updated Nov. 26, 2013, 6:13 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22709
> https://issues.asterisk.org/jira/browse/ASTERISK-22709
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> John Bigelow discovered that if the newly-created atxfer_threeway_nominal 
> test is run in a loop, eventually a test run will cause the UUT instance of 
> Asterisk to crash. Investigation of the crash showed that the crash was being 
> caused while trying to get the string representation of a channel's 
> translation paths while creating a channel snapshot. The attended transfer 
> manager thread was creating the channel snapshot as part of the attended 
> transfer stasis message while in another thread (in this case, the bridge 
> channel thread) the translation paths were being altered to make the channel 
> compatible with others in the bridge.
> 
> It's clear that the issue is that the channel's contents are not lock 
> protected while creating a channel snapshot. Luckily, the channel's lock is 
> held already when updating translation paths. By adding locking, I was able 
> to run the test over 450 times straight with no negative consequences.
> 
> When adding locking, there were two strategies to follow:
> 
> 1) Take advantage of the fact that channel locks are recursive, and just add 
> a single SCOPED_CHANNELLOCK() call to ast_channel_snapshot_create() and call 
> it a day.
> 2) Make an audit of channel-related calls that end up creating a channel 
> snapshot and require locks be held prior to making those calls.
> 
> I like approach 2 more, even though it's harder to pull off. In investigating 
> what paths result in channel snapshots being created, I found that channel 
> locks should be required before calling the following functions:
> 
> * ast_channel_snapshot_create()
> * ast_channel_blob_create()
> * ast_channel_publish_snapshot()
> * ast_publish_channel_state()
> * ast_channel_publish_blob()
> * ast_channel_publish_varset()
> * ast_channel_amaflags_set()
> * ast_channel_callid_set()
> * ast_channel_whentohangup_set()
> * ast_channel_callgroup_set()
> * ast_channel_pickupgroup_set()
> * ast_channel_internal_bridge_set()
> * ast_channel_language_set()
> * ast_channel_accountcode_set()
> * ast_channel_peeraccount_set()
> * ast_channel_linkedid_set()
> * ast_channel_stage_snapshot()
> * ast_setstate()
> * ast_aoc_manager_event()
> * ast_channel_setwhentohangup_tv()
> * ast_channel_setwhentohangup()
> * ast_set_variables()
> 
> All of these functions' documentation have been updated to indicate a 
> precondition of having the channel locked.
> 
> In addition, there are some more complex functions that create channel 
> snapshots but could not easily be made to have the precondition of having the 
> channel locked. These are:
> 
> * ast_bridge_blob_create()
> * ast_publish_attended_transfer_fail()
> * ast_publish_attended_transfer_bridge_merge()
> * ast_publish_attended_transfer_threeway()
> * ast_publish_attended_transfer_app()
> * ast_publish_attended_transfer_link()
> * ast_bridge_publish_enter()
> * ast_bridge_publish_leave()
> * ast_bridge_publish_blind_transfer()
> 
> These functions' documentation now have a precondition of having no channels 
> or bridges locked.
> 
> This review seeks only to make sure that channels are properly locked prior 
> to creating channel snapshots. A similar effort is likely needed to ensure 
> that bridges are locked during bridge snapshot creation.
> 
> 
> Diffs
> -
> 
>   /branches/12/tests/test_stasis_channels.c 402818 
>   /branches/12/tests/test_cel.c 402818 
>   /branches/12/tests/test_cdr.c 402818 
>   /branches/12/res/res_stasis.c 402818 
>   /branches/12/res/res_pjsip_refer.c 402818 
>   /branches/12/res/res_agi.c 402818 
>   /branches/12/res/parking/parking_manager.c 402818 
>   /branches/12/res/parking/parking_bridge_features.c 402818 
>   /branches/12/pbx/pbx_realtime.c 402818 
>   /branches/12/main/stasis_channels.c 402818 
>   /branches/12/main/stasis_bridges.c 402818 
>   /branches/12/main/pickup.c 402818 
>   /branches/12/main/pbx.c 402818 
>   /branches/12/main/endpoints.c 402818 
>   /branches/12/main/dial.c 402818 
>   /branches/12/main/core_unreal.c 402818 
>   /branches/12/main/core_local.c 402818 
>   /branches/12/main/channel.c 402818 
>   /branches/12/main/cel.c 402818 
>   /branches/12/main/bridge_channel.c 402818 
>   /branches/12/main/bridge.c 402818

Re: [asterisk-dev] [Code Review] 2961: Add channel locking for calls that result in channel snapshot creation

2013-12-04 Thread Mark Michelson 

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2961/
---

(Updated Dec. 4, 2013, 3:45 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Bugs: ASTERISK-22709
https://issues.asterisk.org/jira/browse/ASTERISK-22709


Repository: Asterisk


Description
---

John Bigelow discovered that if the newly-created atxfer_threeway_nominal test 
is run in a loop, eventually a test run will cause the UUT instance of Asterisk 
to crash. Investigation of the crash showed that the crash was being caused 
while trying to get the string representation of a channel's translation paths 
while creating a channel snapshot. The attended transfer manager thread was 
creating the channel snapshot as part of the attended transfer stasis message 
while in another thread (in this case, the bridge channel thread) the 
translation paths were being altered to make the channel compatible with others 
in the bridge.

It's clear that the issue is that the channel's contents are not lock protected 
while creating a channel snapshot. Luckily, the channel's lock is held already 
when updating translation paths. By adding locking, I was able to run the test 
over 450 times straight with no negative consequences.

When adding locking, there were two strategies to follow:

1) Take advantage of the fact that channel locks are recursive, and just add a 
single SCOPED_CHANNELLOCK() call to ast_channel_snapshot_create() and call it a 
day.
2) Make an audit of channel-related calls that end up creating a channel 
snapshot and require locks be held prior to making those calls.

I like approach 2 more, even though it's harder to pull off. In investigating 
what paths result in channel snapshots being created, I found that channel 
locks should be required before calling the following functions:

* ast_channel_snapshot_create()
* ast_channel_blob_create()
* ast_channel_publish_snapshot()
* ast_publish_channel_state()
* ast_channel_publish_blob()
* ast_channel_publish_varset()
* ast_channel_amaflags_set()
* ast_channel_callid_set()
* ast_channel_whentohangup_set()
* ast_channel_callgroup_set()
* ast_channel_pickupgroup_set()
* ast_channel_internal_bridge_set()
* ast_channel_language_set()
* ast_channel_accountcode_set()
* ast_channel_peeraccount_set()
* ast_channel_linkedid_set()
* ast_channel_stage_snapshot()
* ast_setstate()
* ast_aoc_manager_event()
* ast_channel_setwhentohangup_tv()
* ast_channel_setwhentohangup()
* ast_set_variables()

All of these functions' documentation have been updated to indicate a 
precondition of having the channel locked.

In addition, there are some more complex functions that create channel 
snapshots but could not easily be made to have the precondition of having the 
channel locked. These are:

* ast_bridge_blob_create()
* ast_publish_attended_transfer_fail()
* ast_publish_attended_transfer_bridge_merge()
* ast_publish_attended_transfer_threeway()
* ast_publish_attended_transfer_app()
* ast_publish_attended_transfer_link()
* ast_bridge_publish_enter()
* ast_bridge_publish_leave()
* ast_bridge_publish_blind_transfer()

These functions' documentation now have a precondition of having no channels or 
bridges locked.

This review seeks only to make sure that channels are properly locked prior to 
creating channel snapshots. A similar effort is likely needed to ensure that 
bridges are locked during bridge snapshot creation.


Diffs
-

  /branches/12/tests/test_stasis_channels.c 402818 
  /branches/12/tests/test_cel.c 402818 
  /branches/12/tests/test_cdr.c 402818 
  /branches/12/res/res_stasis.c 402818 
  /branches/12/res/res_pjsip_refer.c 402818 
  /branches/12/res/res_agi.c 402818 
  /branches/12/res/parking/parking_manager.c 402818 
  /branches/12/res/parking/parking_bridge_features.c 402818 
  /branches/12/pbx/pbx_realtime.c 402818 
  /branches/12/main/stasis_channels.c 402818 
  /branches/12/main/stasis_bridges.c 402818 
  /branches/12/main/pickup.c 402818 
  /branches/12/main/pbx.c 402818 
  /branches/12/main/endpoints.c 402818 
  /branches/12/main/dial.c 402818 
  /branches/12/main/core_unreal.c 402818 
  /branches/12/main/core_local.c 402818 
  /branches/12/main/channel.c 402818 
  /branches/12/main/cel.c 402818 
  /branches/12/main/bridge_channel.c 402818 
  /branches/12/main/bridge.c 402818 
  /branches/12/include/asterisk/stasis_channels.h 402818 
  /branches/12/include/asterisk/stasis_bridges.h 402818 
  /branches/12/include/asterisk/channelstate.h 402818 
  /branches/12/include/asterisk/channel.h 402818 
  /branches/12/include/asterisk/aoc.h 402818 
  /branches/12/funcs/func_timeout.c 402818 
  /branches/12/channels/sig_pri.c 402818 
  /branches/12/channels/sig_analog.c 402818 
  /branches/12/channels/chan_vpb.cc 402818 
  /branches/12/channels/chan_unistim.c 402818 
  /branches/12