[asterisk-commits] schmitds: branch 1.8 r289622 - in /branches: 1.4/channels/ 1.6.2/channels/ 1....

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Fri Oct 1 04:42:30 CDT 2010


Author: schmitds
Date: Fri Oct  1 04:42:22 2010
New Revision: 289622

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=289622
Log:
don't iterate through all dialogs to find and delete old subscribes

On every incoming subscribe there is a iteration through all dialogs to find old subscribes and delete them. This is slow and not RFC conform. This was only needed in 1.2 cause a subscribe was not deleted when a dialog was destroyed, after 1.4 a subscribe get removed when its dialog is destroyed.

(closes issue #17950)
Reported by: schmidts
Tested by: schmidts

Review: https://reviewboard.asterisk.org/r/901/


Modified:
    branches/1.4/channels/chan_sip.c
    branches/1.6.2/channels/chan_sip.c
    branches/1.8/channels/chan_sip.c

Modified: branches/1.4/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.4/channels/chan_sip.c?view=diff&rev=289622&r1=289621&r2=289622
==============================================================================
--- branches/1.4/channels/chan_sip.c (original)
+++ branches/1.4/channels/chan_sip.c Fri Oct  1 04:42:22 2010
@@ -13970,6 +13970,10 @@
 				/* The other side has no transaction to cancel,
 				just assume it's all right then */
 				ast_log(LOG_WARNING, "Remote host can't match request %s to call '%s'. Giving up.\n", sip_methods[sipmethod].text, p->callid);
+			} else if (sipmethod == SIP_NOTIFY) {
+				/* The other side has no active Subscribe for this Callid. 
+				 * Remove this Dialog and old Subscription */
+				ast_set_flag(&p->flags[0], SIP_NEEDDESTROY);	
 			} else {
 				ast_log(LOG_WARNING, "Remote host can't match request %s to call '%s'. Giving up.\n", sip_methods[sipmethod].text, p->callid);
 				/* Guessing that this is not an important request */
@@ -14166,6 +14170,8 @@
 				handle_response_invite(p, resp, rest, req, seqno);
 			} else if (sipmethod == SIP_BYE) {
 				ast_set_flag(&p->flags[0], SIP_NEEDDESTROY);	
+			} else if (sipmethod == SIP_NOTIFY) {
+				ast_set_flag(&p->flags[0], SIP_NEEDDESTROY);	
 			} else if (sipdebug) {
 				ast_log	(LOG_DEBUG, "Remote host can't match request %s to call '%s'. Giving up\n", sip_methods[sipmethod].text, p->callid);
 			}
@@ -16656,7 +16662,6 @@
 				ASTOBJ_UNLOCK(p->relatedpeer);
 			}
 		} else {
-			struct sip_pvt *p_old;
 
 			if ((firststate = ast_extension_state(NULL, p->context, p->exten)) < 0) {
 
@@ -16672,31 +16677,6 @@
 			/* hide the 'complete' exten/context in the refer_to field for later display */
 			ast_string_field_build(p, subscribeuri, "%s@%s", p->exten, p->context);
 
-			/* remove any old subscription from this peer for the same exten/context,
-			as the peer has obviously forgotten about it and it's wasteful to wait
-			for it to expire and send NOTIFY messages to the peer only to have them
-			ignored (or generate errors)
-			*/
-			ast_mutex_lock(&iflock);
-			for (p_old = iflist; p_old; p_old = p_old->next) {
-				if (p_old == p)
-					continue;
-				if (p_old->initreq.method != SIP_SUBSCRIBE)
-					continue;
-				if (p_old->subscribed == NONE)
-					continue;
-				ast_mutex_lock(&p_old->lock);
-				if (!strcmp(p_old->username, p->username)) {
-					if (!strcmp(p_old->exten, p->exten) &&
-					    !strcmp(p_old->context, p->context)) {
-						ast_set_flag(&p_old->flags[0], SIP_NEEDDESTROY);
-						ast_mutex_unlock(&p_old->lock);
-						break;
-					}
-				}
-				ast_mutex_unlock(&p_old->lock);
-			}
-			ast_mutex_unlock(&iflock);
 		}
 		if (!p->expiry)
 			ast_set_flag(&p->flags[0], SIP_NEEDDESTROY);

Modified: branches/1.6.2/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.6.2/channels/chan_sip.c?view=diff&rev=289622&r1=289621&r2=289622
==============================================================================
--- branches/1.6.2/channels/chan_sip.c (original)
+++ branches/1.6.2/channels/chan_sip.c Fri Oct  1 04:42:22 2010
@@ -18641,6 +18641,8 @@
 				handle_response_refer(p, resp, rest, req, seqno);
 			} else if (sipmethod == SIP_SUBSCRIBE) {
 				handle_response_subscribe(p, resp, rest, req, seqno);
+			} else if (sipmethod == SIP_NOTIFY) {
+				pvt_set_needdestroy(p, "received 481 response");
 			} else if (sipmethod == SIP_BYE) {
 				/* The other side has no transaction to bye,
 				just assume it's all right then */
@@ -18840,6 +18842,8 @@
 				handle_response_invite(p, resp, rest, req, seqno);
 			} else if (sipmethod == SIP_BYE) {
 				pvt_set_needdestroy(p, "received 481 response");
+			} else if (sipmethod == SIP_NOTIFY) {
+				pvt_set_needdestroy(p, "received 481 response");
 			} else if (sipdebug) {
 				ast_debug(1, "Remote host can't match request %s to call '%s'. Giving up\n", sip_methods[sipmethod].text, p->callid);
 			}
@@ -21366,7 +21370,6 @@
 	const char *eventheader = get_header(req, "Event");	/* Get Event package name */
 	int resubscribe = (p->subscribed != NONE);
 	char *temp, *event;
-	struct ao2_iterator i;
 
 	if (p->initreq.headers) {	
 		/* We already have a dialog */
@@ -21667,7 +21670,6 @@
 				ao2_unlock(p->relatedpeer);
 			}
 		} else {
-			struct sip_pvt *p_old;
 
 			if ((firststate = ast_extension_state(NULL, p->context, p->exten)) < 0) {
 
@@ -21682,40 +21684,8 @@
 			append_history(p, "Subscribestatus", "%s", ast_extension_state2str(firststate));
 			/* hide the 'complete' exten/context in the refer_to field for later display */
 			ast_string_field_build(p, subscribeuri, "%s@%s", p->exten, p->context);
-
-			/* remove any old subscription from this peer for the same exten/context,
-			as the peer has obviously forgotten about it and it's wasteful to wait
-			for it to expire and send NOTIFY messages to the peer only to have them
-			ignored (or generate errors)
-			*/
-			i = ao2_iterator_init(dialogs, 0);
-			while ((p_old = ao2_t_iterator_next(&i, "iterate thru dialogs"))) {
-				if (p_old == p) {
-					ao2_t_ref(p_old, -1, "toss dialog ptr from iterator_next before continue");
-					continue;
-				}
-				if (p_old->initreq.method != SIP_SUBSCRIBE) {
-					ao2_t_ref(p_old, -1, "toss dialog ptr from iterator_next before continue");
-					continue;
-				}
-				if (p_old->subscribed == NONE) {
-					ao2_t_ref(p_old, -1, "toss dialog ptr from iterator_next before continue");
-					continue;
-				}
-				sip_pvt_lock(p_old);
-				if (!strcmp(p_old->username, p->username)) {
-					if (!strcmp(p_old->exten, p->exten) &&
-					    !strcmp(p_old->context, p->context)) {
-						pvt_set_needdestroy(p_old, "replacing subscription");
-						sip_pvt_unlock(p_old);
-						ao2_t_ref(p_old, -1, "toss dialog ptr from iterator_next before break");
-						break;
-					}
-				}
-				sip_pvt_unlock(p_old);
-				ao2_t_ref(p_old, -1, "toss dialog ptr from iterator_next");
-			}
-			ao2_iterator_destroy(&i);
+			/* Deleted the slow iteration of all sip dialogs to find old subscribes from this peer for exten at context */
+
 		}
 		if (!p->expiry) {
 			pvt_set_needdestroy(p, "forcing expiration");

Modified: branches/1.8/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/channels/chan_sip.c?view=diff&rev=289622&r1=289621&r2=289622
==============================================================================
--- branches/1.8/channels/chan_sip.c (original)
+++ branches/1.8/channels/chan_sip.c Fri Oct  1 04:42:22 2010
@@ -19754,6 +19754,8 @@
 				handle_response_invite(p, resp, rest, req, seqno);
 			} else if (sipmethod == SIP_SUBSCRIBE) {
 				handle_response_subscribe(p, resp, rest, req, seqno);
+			} else if (sipmethod == SIP_NOTIFY) {
+				pvt_set_needdestroy(p, "received 481 response");
 			} else if (sipmethod == SIP_BYE) {
 				/* The other side has no transaction to bye,
 				just assume it's all right then */
@@ -19953,6 +19955,8 @@
 				handle_response_invite(p, resp, rest, req, seqno);
 			} else if (sipmethod == SIP_BYE) {
 				pvt_set_needdestroy(p, "received 481 response");
+			} else if (sipmethod == SIP_NOTIFY) {
+				pvt_set_needdestroy(p, "received 481 response");
 			} else if (sipdebug) {
 				ast_debug(1, "Remote host can't match request %s to call '%s'. Giving up\n", sip_methods[sipmethod].text, p->callid);
 			}
@@ -23011,7 +23015,6 @@
 	const char *eventheader = get_header(req, "Event");	/* Get Event package name */
 	int resubscribe = (p->subscribed != NONE);
 	char *temp, *event;
-	struct ao2_iterator i;
 
 	if (p->initreq.headers) {	
 		/* We already have a dialog */
@@ -23327,8 +23330,6 @@
 				ao2_unlock(p->relatedpeer);
 			}
 		} else if (p->subscribed != CALL_COMPLETION) {
-			struct sip_pvt *p_old;
-
 			if ((firststate = ast_extension_state(NULL, p->context, p->exten)) < 0) {
 
 				ast_log(LOG_NOTICE, "Got SUBSCRIBE for extension %s@%s from %s, but there is no hint for that extension.\n", p->exten, p->context, ast_sockaddr_stringify(&p->sa));
@@ -23342,40 +23343,8 @@
 			append_history(p, "Subscribestatus", "%s", ast_extension_state2str(firststate));
 			/* hide the 'complete' exten/context in the refer_to field for later display */
 			ast_string_field_build(p, subscribeuri, "%s@%s", p->exten, p->context);
-
-			/* remove any old subscription from this peer for the same exten/context,
-			as the peer has obviously forgotten about it and it's wasteful to wait
-			for it to expire and send NOTIFY messages to the peer only to have them
-			ignored (or generate errors)
-			*/
-			i = ao2_iterator_init(dialogs, 0);
-			while ((p_old = ao2_t_iterator_next(&i, "iterate thru dialogs"))) {
-				if (p_old == p) {
-					ao2_t_ref(p_old, -1, "toss dialog ptr from iterator_next before continue");
-					continue;
-				}
-				if (p_old->initreq.method != SIP_SUBSCRIBE) {
-					ao2_t_ref(p_old, -1, "toss dialog ptr from iterator_next before continue");
-					continue;
-				}
-				if (p_old->subscribed == NONE) {
-					ao2_t_ref(p_old, -1, "toss dialog ptr from iterator_next before continue");
-					continue;
-				}
-				sip_pvt_lock(p_old);
-				if (!strcmp(p_old->username, p->username)) {
-					if (!strcmp(p_old->exten, p->exten) &&
-					    !strcmp(p_old->context, p->context)) {
-						pvt_set_needdestroy(p_old, "replacing subscription");
-						sip_pvt_unlock(p_old);
-						ao2_t_ref(p_old, -1, "toss dialog ptr from iterator_next before break");
-						break;
-					}
-				}
-				sip_pvt_unlock(p_old);
-				ao2_t_ref(p_old, -1, "toss dialog ptr from iterator_next");
-			}
-			ao2_iterator_destroy(&i);
+			/* Deleted the slow iteration of all sip dialogs to find old subscribes from this peer for exten at context */
+
 		}
 		if (!p->expiry) {
 			pvt_set_needdestroy(p, "forcing expiration");




More information about the asterisk-commits mailing list