[asterisk-commits] murf: branch murf/bug11210 r114040 - /team/murf/bug11210/channels/chan_sip.c

[SVN commits to the Asterisk project]

Author: murf
Date: Thu Apr 10 13:30:01 2008
New Revision: 114040

URL: http://svn.digium.com/view/asterisk?view=rev&rev=114040
Log:
mostly documentation, formatting fixes; the main fix is the return of zero from the dialog_needdestroy() func instead of CMP_STOP. That little flub completely constipated asterisk's ability to destroy dialogs and free up channels. Usually, you'd just die after you ran out of file descriptors.

Modified:
    team/murf/bug11210/channels/chan_sip.c

Modified: team/murf/bug11210/channels/chan_sip.c
URL: http://svn.digium.com/view/asterisk/team/murf/bug11210/channels/chan_sip.c?view=diff&rev=114040&r1=114039&r2=114040
==============================================================================
--- team/murf/bug11210/channels/chan_sip.c (original)
+++ team/murf/bug11210/channels/chan_sip.c Thu Apr 10 13:30:01 2008
@@ -1340,7 +1340,7 @@
 static struct sip_pvt *dialog_ref_debug(struct sip_pvt *p, char *tag, char *file, int line, const char *func)
 {
 	if (p)
-		ao2_ref_debug(p, 1, tag, file, line, func);
+		_ao2_ref_debug(p, 1, tag, file, line, func);
 	else
 		ast_log(LOG_ERROR, "Attempt to Ref a null pointer\n");
 	return p;
@@ -1349,7 +1349,7 @@
 static struct sip_pvt *dialog_unref_debug(struct sip_pvt *p, char *tag, char *file, int line, const char *func)
 {
 	if (p)
-		ao2_ref_debug(p, -1, tag, file, line, func);
+		_ao2_ref_debug(p, -1, tag, file, line, func);
 	return NULL;
 }
 #else
@@ -3063,6 +3063,7 @@
 
 	if (xmitres == XMIT_ERROR) {	/* Serious network trouble, no need to try again */
 		append_history(pkt->owner, "XmitErr", "%s", pkt->is_fatal ? "(Critical)" : "(Non-critical)");
+		ast_log(LOG_ERROR, "Serious Network Trouble; __sip_xmit returns error for pkt data\n");
 		if (pkt->data)
 			ast_free(pkt->data);
 		return AST_FAILURE;
@@ -12911,16 +12912,34 @@
    are marked needdestroy. It will return CMP_MATCH for candidates, and they
    will be unlinked */
 
+/* TODO: Implement a queue and instead of marking a dialog 
+   to be destroyed, toss it into the queue. Have a separate
+   thread do the locking and destruction */
+
 static int dialog_needdestroy(void *dialogobj, void *arg, int flags) 
 {
 	struct sip_pvt *dialog = dialogobj;
 	time_t *t = arg;
 	
 	if (sip_pvt_trylock(dialog)) {
+		/* this path gets executed fairly frequently (3% or so) even in low load
+		   situations; the routines we most commonly fight for a lock with:
+		   sip_answer (7 out of 9)
+		   sip_hangup (2 out of 9)
+		*/
 		ao2_unlock(dialogs);
 		usleep(1);
 		ao2_lock(dialogs);
-		return CMP_STOP;
+		/* I had previously returned CMP_STOP here; but changed it to return
+		   a zero instead, because there is no need to stop at the first sign
+		   of trouble. The old algorithm would restart in such circumstances,
+		   but there is no need for this now in astobj2-land. We'll
+		   just skip over this element this time, and catch it on the
+		   next traversal, which is about once a second or so. See the 
+		   ao2_callback call in do_monitor. We don't want the number of
+		   dialogs to back up too much between runs.
+		*/
+		return 0;
 	}
 	
 	/* Check RTP timeouts and kill calls if we have a timeout set and do not get RTP */
@@ -12934,7 +12953,6 @@
 		/* no, the unlink should handle this: dialog_unref(dialog, "needdestroy: one more refcount decrement to allow dialog to be destroyed"); */
 		/* the CMP_MATCH will unlink this dialog from the dialog hash table */
 		dialog_unlink_all(dialog, TRUE, FALSE);
-		
 		return 0; /* the unlink_all should unlink this from the table, so.... no need to return a match */
 	}
 	sip_pvt_unlock(dialog);
@@ -18220,8 +18238,9 @@
 	struct ast_channel *bridged_to;
 	
 	/* If we have an INCOMING invite that we haven't answered, terminate that transaction */
-	if (p->pendinginvite && !ast_test_flag(&p->flags[0], SIP_OUTGOING) && !req->ignore && !p->owner) 
+	if (p->pendinginvite && !ast_test_flag(&p->flags[0], SIP_OUTGOING) && !req->ignore && !p->owner) {
 		transmit_response_reliable(p, "487 Request Terminated", &p->initreq);
+	}
 
 	p->invitestate = INV_TERMINATED;
 
@@ -18287,7 +18306,7 @@
 		ast_debug(3, "Received bye, issuing owner hangup\n");
 	} else {
 		sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT);
-			ast_debug(3, "Received bye, no owner, selfdestruct soon.\n");
+		ast_debug(3, "Received bye, no owner, selfdestruct soon.\n");
 	}
 	transmit_response(p, "200 OK", req);
 
@@ -19326,7 +19345,6 @@
 		   of time since the last time we did it (when MWI is being sent, we can
 		   get back to this point every millisecond or less)
 		*/
-
 		ao2_t_callback(dialogs, OBJ_UNLINK|OBJ_NODATA, dialog_needdestroy, &t, "callback to remove dialogs w/needdestroy");
 
 		/* the old methodology would be to restart the search for dialogs to delete with every