[asterisk-commits] russell: trunk r77889 - in /trunk: ./ channels/chan_iax2.c

[SVN commits to the Asterisk project]

Author: russell
Date: Wed Aug  1 17:24:23 2007
New Revision: 77889

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

........
r77887 | russell | 2007-08-01 17:16:17 -0500 (Wed, 01 Aug 2007) | 23 lines

Fix some race conditions which have been causing weird problems in chan_iax2.
The most notable problem is that people have been seeing storms of VNAK frames
being sent due to really old frames mysteriously being in the retransmission
queue and never getting removed.

It was possible that a dynamic thread got created, but did not acquire its lock
before the thread that created it signals it to perform an action.  When this
happens, the thread will sleep until it hits a timeout, and then get destroyed.
So, the action never gets performed and in some cases, means a frame doesn't
get transmitted and never gets freed since the scheduler never gets a chance
to reschedule transmission.

Another less severe race condition is in the handling of a timeout for a dynamic
thread.  It was possible for it to be acquired to perform at action at the same
time that it hit a timeout.  When this occurs, whatever action it was acquired
for would never get performed.

(patch contributed by Mihai and SteveK)
(closes issue #10289)
(closes issue #10248)
(closes issue #10232)
(possibly related to issue #10359)

........

Modified:
    trunk/   (props changed)
    trunk/channels/chan_iax2.c

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

Modified: trunk/channels/chan_iax2.c
URL: http://svn.digium.com/view/asterisk/trunk/channels/chan_iax2.c?view=diff&rev=77889&r1=77888&r2=77889
==============================================================================
--- trunk/channels/chan_iax2.c (original)
+++ trunk/channels/chan_iax2.c Wed Aug  1 17:24:23 2007
@@ -745,6 +745,7 @@
 	time_t checktime;
 	ast_mutex_t lock;
 	ast_cond_t cond;
+	unsigned int ready_for_signal:1;
 	/*! if this thread is processing a full frame,
 	  some information about that frame will be stored
 	  here, so we can avoid dispatching any more full
@@ -1042,13 +1043,16 @@
 		ast_cond_destroy(&thread->cond);
 		ast_mutex_destroy(&thread->lock);
 		ast_free(thread);
-		thread = NULL;
+		return NULL;
 	}
 
 	/* this thread is not processing a full frame (since it is idle),
 	   so ensure that the field for the full frame call number is empty */
-	if (thread)
-		memset(&thread->ffinfo, 0, sizeof(thread->ffinfo));
+	memset(&thread->ffinfo, 0, sizeof(thread->ffinfo));
+
+	/* Wait for the thread to be ready before returning it to the caller */
+	while (!thread->ready_for_signal)
+		usleep(1);
 
 	return thread;
 }
@@ -8369,11 +8373,15 @@
 		/* Wait for something to signal us to be awake */
 		ast_mutex_lock(&thread->lock);
 
+		/* Flag that we're ready to accept signals */
+		thread->ready_for_signal = 1;
+		
 		/* Put into idle list if applicable */
 		if (put_into_idle)
 			insert_idle_thread(thread);
 
 		if (thread->type == IAX_THREAD_TYPE_DYNAMIC) {
+			struct iax2_thread *t = NULL;
 			/* Wait to be signalled or time out */
 			tv = ast_tvadd(ast_tvnow(), ast_samp2tv(30000, 1000));
 			ts.tv_sec = tv.tv_sec;
@@ -8381,15 +8389,19 @@
 			if (ast_cond_timedwait(&thread->cond, &thread->lock, &ts) == ETIMEDOUT) {
 				ast_mutex_unlock(&thread->lock);
 				AST_LIST_LOCK(&dynamic_list);
-				AST_LIST_REMOVE(&dynamic_list, thread, list);
+				/* Account for the case where this thread is acquired *right* after a timeout */
+				if ((t = AST_LIST_REMOVE(&dynamic_list, thread, list)))
+					ast_atomic_fetchadd_int(&iaxdynamicthreadcount, -1);
 				AST_LIST_UNLOCK(&dynamic_list);
-				ast_atomic_dec_and_test(&iaxdynamicthreadcount);
-				break;		/* exiting the main loop */
+				if (t)
+					break;		/* exiting the main loop */
 			}
+			if (!t)
+				ast_mutex_unlock(&thread->lock);
 		} else {
 			ast_cond_wait(&thread->cond, &thread->lock);
-		}
-		ast_mutex_unlock(&thread->lock);
+			ast_mutex_unlock(&thread->lock);
+		}
 
 		/* Add ourselves to the active list now */
 		AST_LIST_LOCK(&active_list);