[asterisk-dev] [Code Review] Remove potential deadlock in call pickup race.

[rmudgett]

> On 2011-05-27 07:06:22, Alec Davis wrote:
> > Could never get the next 3 senarios to fail, with or without this review patch.
1 call with 2 ringing extensions in same group, with 2 extensions concurrently attempt pickup.
1 call with 1 ringing extension, with 2 extensions concurrently attempt pickup.
2 DAHDI extensions ringing in to the same call group with each destination with 2 ringing calls, and with 2 extensions attempt pickup. Both calls were picked up as expected.

Which indicates, all is well with the above senarios.

But I still wonder what's the particluar senario that fails.

Deadlock is possible when holding the target channel lock and trying to get the chan channel lock.  The patch removes needing to hold the target lock after adding the datastore and getting the connected line data off of the target channel.

Holding the target lock when calling ast_channel_masquerade() is not a good idea because that routine does deadlock avoidance.  (There is legacy code elsewhere calling that function with locks held.)


- rmudgett


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


On 2011-05-26 18:31:19, rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1234/
> -----------------------------------------------------------
> 
> (Updated 2011-05-26 18:31:19)
> 
> 
> Review request for Asterisk Developers, Alec Davis and irroot.
> 
> 
> Summary
> -------
> 
> Fix potential deadlock in ast_do_pickup() with channels chan and target lock and channel container lock.
> 
> This should eliminate the deadlock avoidance code in https://reviewboard.asterisk.org/r/1118/ for issue 18830.
> 
> * Moved can_pickup() to ast_can_pickup() in features.c.
> 
> 
> This addresses bugs 18654 and 18830.
>     https://issues.asterisk.org/view.php?id=18654
>     https://issues.asterisk.org/view.php?id=18830
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/include/asterisk/features.h 321209 
>   /branches/1.8/main/features.c 321209 
>   /branches/1.8/apps/app_directed_pickup.c 321209 
> 
> Diff: https://reviewboard.asterisk.org/r/1234/diff
> 
> 
> Testing
> -------
> 
> Tested a simple call pickup to make sure that it at least still works.
> Needs competing call pickup testing.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110527/ea20f4f8/attachment.htm>