[asterisk-dev] Another *8 deadlock :(

[Gregory Nietsky]

Steve found this serves me well ....




Index: main/channel.c
===================================================================
--- main/channel.c      (.../trunk)     (revision 321557)
+++ main/channel.c      (.../team/irroot/distrotech-customers-trunk)    
(revision 321557)
@@ -6493,8 +6501,12 @@
          */
         ao2_lock(channels);

-       /* lock the original channel to determine if the masquerade is 
require or not */
-       ast_channel_lock(original);
+       /* lock the original channel with deadlock avoidance to 
determine if the masquerade is require or not */
+       while (ast_channel_trylock(original)) {
+               ao2_unlock(channels);
+               usleep(1);
+               ao2_lock(channels);
+       }

On 02/06/2011 18:34, Steve Davies wrote:
> I will immediately confess that this deadlock only exists in pre-1.8
> code, but pretty please could I have an opinion on whether it is safe
> to unlock and relock the 'original' channel in
> ast_do_masquerade(original) in order to avoid a potential deadlock?
>
> The deadlock is quite easily caused on our test rig, but the use of
> ao2 channel refs in 1.8+ solves it quite elegantly there.
> Unfortunately I have a number of older systems that I cannot yet
> migrate :(
>
> Basically 99% of the code will get the channel-list lock before a
> channel lock. ast_do_masquerade() tries to do the opposite. If this
> patch is considered safe, I'll raise it as an issue so that the
> licence can be flagged properly on the patch.
>
> Thanks,
> Steve
>
>
> # diff -up main/channel.c~ main/channel.c
> --- main/channel.c     2011-06-02 17:23:30.000000000 +0100
> +++ main/channel.c      2011-06-02 17:23:09.000000000 +0100
> @@ -5072,7 +5072,10 @@ int ast_do_masquerade(struct ast_channel
>                          clonechan->hangupcause,
>                          ast_cause2str(clonechan->hangupcause)
>                          );
> +               /* Cannot hold a channel lock when calling
> ast_channel_free() as it uses the channel-list lock */
> +               ast_channel_unlock(original);
>                  ast_channel_free(clonechan);
> +               ast_channel_lock(original);
>          } else {
>                  ast_debug(1, "Released clone lock on '%s'\n", clonechan->name);
>                  ast_set_flag(clonechan, AST_FLAG_ZOMBIE);
>
> --
> _____________________________________________________________________
> -- Bandwidth and Colocation Provided by http://www.api-digital.com --
>
> asterisk-dev mailing list
> To UNSUBSCRIBE or update options visit:
>     http://lists.digium.com/mailman/listinfo/asterisk-dev