[asterisk-dev] [Code Review] Properly handle the linkedid for local channels and fix race condition for LINKEDID_END CEL event.

[Mark Michelson]

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



/branches/1.8/main/cel.c
<https://reviewboard.asterisk.org/r/1895/#comment11231>

    This comment could use a bit more clarity. Instead of saying "use" here, indicate that you mean a reference for each channel with this linkedid. It took longer than I care to admit to understand this.



/branches/1.8/main/cel.c
<https://reviewboard.asterisk.org/r/1895/#comment11229>

    Check for failure.



/branches/1.8/main/cel.c
<https://reviewboard.asterisk.org/r/1895/#comment11230>

    These two failure cases should ao2_ref(linkedids, -1);


- Mark


On May 2, 2012, 8:38 a.m., Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1895/
> -----------------------------------------------------------
> 
> (Updated May 2, 2012, 8:38 a.m.)
> 
> 
> Review request for Asterisk Developers and Mark Michelson.
> 
> 
> Summary
> -------
> 
> While writing CEL tests, I noticed that if there is no requestor channel that the ;1 and ;2 sides of a local channel would have different linkedids when they should be the same. After fixing that issue, I found that Local channels were hung up so close together that the time between unlinking and destroying the channel would cause a race condition giving me 0, 1, or 2 LINKEDID_END events based on when they were unlinked.
> 
> This patch has the ;2 channel inherit the linkedid of the ;1 channel and fixes the race condition by no longer scanning the channel list for "other" channels with the same linkedid. Instead, cel.c has an ao2 container of linkedid strings and uses the refcount of the string as a counter of how many channels with the linkedid exist. Not only does this eliminate the race condition, but it also allows us to look up the linkedid by the hashed key instead of traversing the entire channel list.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/channels/chan_local.c 365004 
>   /branches/1.8/main/cel.c 365004 
> 
> Diff: https://reviewboard.asterisk.org/r/1895/diff
> 
> 
> Testing
> -------
> 
> Ran through the dialplan in the cdr/cdr_accountcode testsuite test and tested a single-channel call. Only one linkedid appeared, no refcount errors.
> 
> 
> Thanks,
> 
> Terry
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120502/18cb4697/attachment-0001.htm>