[asterisk-bugs] [JIRA] (ASTERISK-17323) manager_park can deadlock with ast_channel_free, for channel 1 of the park operation (channel list v channel lock)

[Matt Jordan (JIRA)]

    [ https://issues.asterisk.org/jira/browse/ASTERISK-17323?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=216111#comment-216111 ] 

Matt Jordan commented on ASTERISK-17323:
----------------------------------------

Hey David -

So, I know there wasn't much feedback on this issue, but looking at the current Asterisk 1.8 code for {{manager_park}}, this should not longer be an issue. {{ast_channel_get_by_name}} no longer has a locking order inversion with {{ast_channel_free}}. I'm going to go ahead and close this out as "Fixed" in some release of 1.8 and later.

I know that obviously doesn't help the system that this was reported on, but since those versions didn't have a lot of the architectural changes that were made in 1.8, it was very difficult to fix it correctly in those versions.

Matt

> manager_park can deadlock with ast_channel_free, for channel 1 of the park operation (channel list v channel lock)
> ------------------------------------------------------------------------------------------------------------------
>
>                 Key: ASTERISK-17323
>                 URL: https://issues.asterisk.org/jira/browse/ASTERISK-17323
>             Project: Asterisk
>          Issue Type: Bug
>          Components: Channels/General
>            Reporter: David Woolley
>
> manager_park (in features.c) does ast_get_channel_by_name_locked on both channels that it is given.  This locks the list of channels during its processing, meaning that the manager thread ends up locking channel 1, and then the list of channels.
> ast_channel_free write locks the list of channels, then temporarily locks the channel it is freeing, meaning it applies lock in the reverse order!
> A real deadlock has been observed with manager_park waiting for channel list and ast_channel_free waiting for the channel.
> ****** ADDITIONAL INFORMATION ******
> This was was noticed on 1.6.1.0, but the source code is unchanged in this area in 1.6.2 branch.  There has been significant restructuring of channel locking in trunk and I haven't analysed that to see if it fixes the problem.
> The locking logic in ast_channel_free was introduced in revision 219136, to fix ASTERISK-14305.
> Abstract from channel.c (ast_channel_free), with blame information:
> 219194 mnicholson               AST_RWLIST_WRLOCK(&channels);
> 219194 mnicholson               if (!AST_RWLIST_REMOVE(&channels, chan, chan_lis
> t)) {
> 219194 mnicholson                       ast_debug(1, "Unable to find channel in list to free. Assuming it has already been done.\n");
> 219194 mnicholson               }
> 219194 mnicholson               /* Lock and unlock the channel just to be sure nobody has it locked still
> 219194 mnicholson                  due to a reference retrieved from the channel list. */
> 219194 mnicholson               ast_channel_lock(chan);
> 219194 mnicholson               ast_channel_unlock(chan);
> Abstract from features.c (manager_park):
>  12163   tilghman       ch1 = ast_get_channel_by_name_locked(channel);
>  12163   tilghman       if (!ch1) {
>  12163   tilghman               snprintf(buf, sizeof(buf), "Channel does not exist: %s", channel);
>  12163   tilghman               astman_send_error(s, m, buf);
>  12163   tilghman               return 0;
>  12163   tilghman       }
>  12163   tilghman 
>  12163   tilghman       ch2 = ast_get_channel_by_name_locked(channel2);



--
This message was sent by Atlassian JIRA
(v6.2#6252)