[svn-commits] dvossel: trunk r259870 - in /trunk: ./ channels/chan_local.c main/channel.c

[SVN commits to the Digium repositories]

Author: dvossel
Date: Wed Apr 28 16:20:03 2010
New Revision: 259870

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=259870
Log:
Merged revisions 259858 via svnmerge from 
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
  r259858 | dvossel | 2010-04-28 16:16:03 -0500 (Wed, 28 Apr 2010) | 33 lines
  
  resolves deadlocks in chan_local
  
  Issue_1.
  In the local_hangup() 3 locks must be held at the same time... pvt, pvt->chan,
  and pvt->owner.  Proper deadlock avoidance is done when the channel to hangup
  is the outbound chan_local channel, but when it is not the outbound channel we
  have an issue... We attempt to do deadlock avoidance only on the tech pvt, when
  both the tech pvt and the pvt->owner are locked coming into that loop.  By
  never giving up the pvt->owner channel deadlock avoidance is not entirely possible.
  This patch resolves that by doing deadlock avoidance on both the pvt->owner and the pvt
  when trying to get the pvt->chan lock.
  
  Issue_2.
  ast_prod() is used in ast_activate_generator() to queue a frame on the channel
  and make the channel's read function get called.  This function is used in
  ast_activate_generator() while the channel is locked, which mean's the channel
  will have a lock both from the generator code and the frame_queue code by the
  time it gets to chan_local.c's local_queue_frame code... local_queue_frame
  contains some of the same crazy deadlock avoidance that local_hangup requires,
  and this recursive lock prevents that deadlock avoidance from happening correctly.
  This patch removes ast_prod() from the channel lock so only one lock is held during
  the local_queue_frame function.
  
  (closes issue #17185)
  Reported by: schmoozecom
  Patches:
        issue_17185_v1.diff uploaded by dvossel (license 671)
        issue_17185_v2.diff uploaded by dvossel (license 671)
  Tested by: schmoozecom, GameGamer43
  
  Review: https://reviewboard.asterisk.org/r/631/
........

Modified:
    trunk/   (props changed)
    trunk/channels/chan_local.c
    trunk/main/channel.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-1.4-merged' - no diff available.

Modified: trunk/channels/chan_local.c
URL: http://svnview.digium.com/svn/asterisk/trunk/channels/chan_local.c?view=diff&rev=259870&r1=259869&r2=259870
==============================================================================
--- trunk/channels/chan_local.c (original)
+++ trunk/channels/chan_local.c Wed Apr 28 16:20:03 2010
@@ -667,12 +667,12 @@
 			/* Deadlock avoidance */
 			while (p->owner && ast_channel_trylock(p->owner)) {
 				ast_mutex_unlock(&p->lock);
-				if (ast) {
-					ast_channel_unlock(ast);
+				if (p->chan) {
+					ast_channel_unlock(p->chan);
 				}
 				usleep(1);
-				if (ast) {
-					ast_channel_lock(ast);
+				if (p->chan) {
+					ast_channel_lock(p->chan);
 				}
 				ast_mutex_lock(&p->lock);
 			}
@@ -687,8 +687,17 @@
 	} else {
 		ast_module_user_remove(p->u_owner);
 		while (p->chan && ast_channel_trylock(p->chan)) {
-			DEADLOCK_AVOIDANCE(&p->lock);
-		}
+				ast_mutex_unlock(&p->lock);
+				if (p->owner) {
+					ast_channel_unlock(p->owner);
+				}
+				usleep(1);
+				if (p->owner) {
+					ast_channel_lock(p->owner);
+				}
+				ast_mutex_lock(&p->lock);
+		}
+
 		p->owner = NULL;
 		if (p->chan) {
 			ast_queue_hangup(p->chan);

Modified: trunk/main/channel.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/channel.c?view=diff&rev=259870&r1=259869&r2=259870
==============================================================================
--- trunk/main/channel.c (original)
+++ trunk/main/channel.c Wed Apr 28 16:20:03 2010
@@ -2544,24 +2544,21 @@
 	int res = 0;
 
 	ast_channel_lock(chan);
-
 	if (chan->generatordata) {
 		if (chan->generator && chan->generator->release)
 			chan->generator->release(chan, chan->generatordata);
 		chan->generatordata = NULL;
 	}
-
-	ast_prod(chan);
 	if (gen->alloc && !(chan->generatordata = gen->alloc(chan, params))) {
 		res = -1;
 	}
-	
 	if (!res) {
 		ast_settimeout(chan, 50, generator_force, chan);
 		chan->generator = gen;
 	}
-
 	ast_channel_unlock(chan);
+
+	ast_prod(chan);
 
 	return res;
 }