[asterisk-dev] [Code Review] Fix a deadlock in agents occuring due to trying to lock a channel while having a lock on the pvt.

[rmudgett]

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


To be paranoid you should check that owner and p->owner still point to the same place after reacquiring the locks.  If they differ then you need to start over and acquire the correct owner and lock.

Also since this code seems to be inlined now in a bunch of places should you not consider an agent_lock_all(*pvt) that will return a ref to the owner as well as locking it?


/branches/1.8/channels/chan_agent.c
<https://reviewboard.asterisk.org/r/1708/#comment9966>

    An unlock p->lock is missing here.


- rmudgett


On Feb. 3, 2012, 8:57 a.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1708/
> -----------------------------------------------------------
> 
> (Updated Feb. 3, 2012, 8:57 a.m.)
> 
> 
> Review request for Asterisk Developers, Mark Michelson, rmudgett, and Matt Jordan.
> 
> 
> Summary
> -------
> 
> Also adds locking to a number of other functions which are calling ast_bridged_channel (which is documented as requiring a lock for safe running, which was the purpose of irroot's original locking patch in action_agents.
> 
> Unlike the other patch on reviewboard right now, this opts to enforce locking order instead of using deadlock avoidance.
> 
> Celebrity endorsement: "This looks incredibly sane to me." - MMichelson
> 
> 
> This addresses bug ASTERISK-19285.
>     https://issues.asterisk.org/jira/browse/ASTERISK-19285
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/channels/chan_agent.c 353685 
> 
> Diff: https://reviewboard.asterisk.org/r/1708/diff
> 
> 
> Testing
> -------
> 
> Not much I'm afraid to say. I can't reproduce the issue since it involves real world use over a period of time.  I'll ask Alex Villacís Lasso to give it a shot though.
> 
> 
> Thanks,
> 
> jrose
> 
>

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