[asterisk-dev] [Code Review] Fix deadlock when using originate from agent

[Jeff Peeler]

> On 2008-11-11 16:19:17, Tilghman Lesher wrote:
> > branches/1.4/channels/chan_agent.c, line 944
> > <http://reviewboard.digium.com/r/35/diff/1/?file=1248#file1248line944>
> >
> >     I'm curious as to why you lock before signalling.  The manpage suggests that this is not strictly necessary.  Is it only to set p->app_lock_flag ?  See my other comment, as it appears that this variable is useless.

You're right - I locked in a bunch of unnecessary places I believe so I'm going to remove them.


> On 2008-11-11 16:19:17, Tilghman Lesher wrote:
> > branches/1.4/channels/chan_agent.c, line 1088
> > <http://reviewboard.digium.com/r/35/diff/1/?file=1248#file1248line1088>
> >
> >     How can this ever be true, since you've just set p->app_lock_flag to 1 on the previous line?

Well, I thought that it might be possible for another thread to set it to 0 immediately afterwards. That isn't the proper way of copying the old trylock behavior. I think I have it correct now.


- Jeff


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/35/#review119
-----------------------------------------------------------


On 2008-11-06 17:11:31, Jeff Peeler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/35/
> -----------------------------------------------------------
> 
> (Updated 2008-11-06 17:11:31)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This changes the behavior of __login_exec to use a pthread condition to wait rather than just waiting for a lock. Using a pthread condition frees the restraint of requiring the same thread to let execution continue (by unlocking the lock), which is important as originate uses multiple threads.
> 
> 
> This addresses bug 12269.
>     http://bugs.digium.com/view.php?id=12269
> 
> 
> Diffs
> -----
> 
>   branches/1.4/channels/chan_agent.c 154966 
> 
> Diff: http://reviewboard.digium.com/r/35/diff
> 
> 
> Testing
> -------
> 
> Testing done both here and by reporters show the problem is resolved, but I wanted to make sure I'm not missing anything as changing locking can be tricky.
> 
> 
> Thanks,
> 
> Jeff
> 
>