[asterisk-dev] [Code Review] Deadlock ao2_callback / ast_write / handle_incoming

[astmiv]

> On 2011-04-13 04:03:40, astmiv wrote:
> > It appears that locking a pvt is done without checking if the owner of the pvt is also locked. I have reported another issue 19107 which has the same deadlock problem with one other different location.
> > 
> > One thread is first locking the channel and then would like to lock pvt while another first locks the pvt and then would like to lock the channel.
> > 
> > When I read through the code in sip_chan.c i conclude that the channel (pvt->owner) should always be locked first before locking the pvt. Is this right?
> > 
> > I think we should make a solution which fixes this in every possible situation.

/* since p->owner (c) is unlocked, we need to go ahead and unlock pvt for both
			 * magic pickup and ast_hangup.  Both of these functions will attempt to lock
			 * p->owner again, which can cause a deadlock if we already hold a lock on p.
			 * Locking order is, channel then pvt.  Dead lock avoidance must be used if
			 * called the other way around. */

>From sip_chan.c in function handle_request_invite()


- astmiv


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


On 2011-04-13 03:17:24, irroot wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1171/
> -----------------------------------------------------------
> 
> (Updated 2011-04-13 03:17:24)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> 
> Channel walk is contending a lock on a channel held by ast_write while sip_write is contending a private lock held by handle incoming that is contending the channel lock held by the write ...
> 
> now if i was holding the channel lock in handle_incoming before i locked private this would not happen ... 
> 
> some how ive been appointed the deadlock fairy ... monitoring +/- 100 1.8 sites for deadlocks and attempt to fix them as they
> popup.
> 
> 
> === Thread ID: 0xb3cf7b70 (do_devstate_changes started at [ 724] devicestate.c ast_device_state_engine_init())
> === ---> Lock #0 (astobj2.c): MUTEX 657 internal_ao2_callback c 0x85fe1b8 (1)
> === ---> Waiting for Lock #1 (channel.c): MUTEX 1641 ast_channel_cmp_cb chan 0xabb7a558 (1)
> === --- ---> Locked Here: channel.c line 4690 (ast_write)
> === -------------------------------------------------------------------
> ===
> === Thread ID: 0xabf3fb70 (do_monitor started at [24621] chan_sip.c restart_monitor())
> === ---> Lock #0 (chan_sip.c): MUTEX 24593 do_monitor &monlock 0xb314fde0 (1)
> === ---> Lock #1 (chan_sip.c): MUTEX 23781 handle_incoming p 0xab7c94a8 (1)
> === ---> Waiting for Lock #2 (channel.c): MUTEX 1361 __ast_queue_frame chan 0xabb7a558 (1)
> === --- ---> Locked Here: channel.c line 4690 (ast_write)
> === -------------------------------------------------------------------
> ===
> === Thread ID: 0xab6c3b70 (pbx_thread started at [ 5038] pbx.c ast_pbx_start())
> === ---> Lock #0 (channel.c): MUTEX 4690 ast_write chan 0xabb7a558 (1)
> === ---> Waiting for Lock #1 (chan_sip.c): MUTEX 6083 sip_write p 0xab7c94a8 (1)
> === --- ---> Locked Here: chan_sip.c line 23781 (handle_incoming)
> === -------------------------------------------------------------------
> === 
> 
> 
> This addresses bug 19112.
>     https://issues.asterisk.org/view.php?id=19112
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/channels/chan_sip.c 313425 
> 
> Diff: https://reviewboard.asterisk.org/r/1171/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> irroot
> 
>

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