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

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Tue Dec 18 10:01:44 CST 2007


Author: murf
Date: Tue Dec 18 10:01:43 2007
New Revision: 93624

URL: http://svn.digium.com/view/asterisk?view=rev&rev=93624
Log:
I added dialog_ref() and dialog_unref() calls in the dialog waitid ast_sched_xxx calls. It seems to me disturbing if a dialog were to disappear while a sched event was still waiting to happen. Probably wouldn't normally happen anyway, but just in case....  Also, was previously suffering from lockups in the unload_module stuff, but maybe the previous fixes concerning dialog callid changes might have also cleared up this problem as well. Will leave the debugs in place until I'm more sure.

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=93624&r1=93623&r2=93624
==============================================================================
--- team/murf/bug11210/channels/chan_sip.c (original)
+++ team/murf/bug11210/channels/chan_sip.c Tue Dec 18 10:01:43 2007
@@ -1559,7 +1559,6 @@
 static int dialog_cmp_cb(void *obj, void *arg, int flags)
 {
 	struct sip_pvt *pvt = obj, *pvt2 = arg;
-	ast_log(LOG_NOTICE, "dialog_cmp_cb(%s,%s) ? %d\n", pvt->callid, pvt2->callid, !strcasecmp(pvt->callid, pvt2->callid) ? CMP_MATCH : 0);
 	
 	return !strcasecmp(pvt->callid, pvt2->callid) ? CMP_MATCH : 0;
 }
@@ -3226,11 +3225,13 @@
 	}
 	if (peer->expire > -1) {
 		ast_log(LOG_WARNING,"About to sched_del peer->expire = %d in __sip_destroy_peer\n", peer->expire);
-		ast_sched_del(sched, peer->expire);
+		ast_sched_del(sched, peer->expire); /* HUH?  normally, if peer were being refcounted thru sched calls, we'd
+											   add code after this to unref the peer */
 	}
 	
 	if (peer->pokeexpire > -1)
-		ast_sched_del(sched, peer->pokeexpire);
+		ast_sched_del(sched, peer->pokeexpire); /* HUH?  normally, if peer were being refcounted thru sched calls, we'd
+											   add code after this to unref the peer */
 	register_peer_exten(peer, FALSE);
 	ast_free_ha(peer->ha);
 	if (peer->selfdestruct)
@@ -3426,7 +3427,9 @@
 		if (ast_test_flag(&global_flags[1], SIP_PAGE2_RTAUTOCLEAR)) {
 			ast_log(LOG_WARNING,"About to sched_replace peer->expire = %d in realtime_peer\n", peer->expire);
 			peer->expire = ast_sched_replace(peer->expire, sched, 
-				global_rtautoclear * 1000, expire_register, (void *) peer);
+				global_rtautoclear * 1000, expire_register, (void *) peer); /* HUH? peer is a refcounted object, and we are storing it in the sched struct, so 
+																			   really, really, we should be incr. its refcount right here, but I guess, since
+																			   peers hang around until module unload time anyway, it's not worth the trouble */
 		}
 		ao2_link(peers, peer, "link peer into peers table");
 		ao2_link(peers_by_ip, peer, "link peer into peers_by_ip table");
@@ -3940,12 +3943,14 @@
 	}
 	if (reg->expire > -1) {
 		ast_log(LOG_WARNING,"About to sched_del reg->expire in sip_registry_destroy\n");
-		ast_sched_del(sched, reg->expire);
+		ast_sched_del(sched, reg->expire);  /* HUH?  normally, if reg were being refcounted thru sched calls, we'd
+											   add code after this to unref the reg */
 	}
 	
 	if (reg->timeout > -1) {
 		ast_log(LOG_WARNING,"About to sched_del reg->timeout in sip_registry_destroy\n");
-		ast_sched_del(sched, reg->timeout);
+		ast_sched_del(sched, reg->timeout);   /* HUH?  normally, if peer were being refcounted thru sched calls, we'd
+											   add code after this to unref the peer */
 	}
 	
 	ast_string_field_free_memory(reg);
@@ -3990,7 +3995,9 @@
 	if (p->waitid > -1)
 	{
 		ast_log(LOG_WARNING,"About to sched_del waitid(%d) for dialog %s in __sip_destroy\n", p->waitid, p->callid);
-		ast_sched_del(sched, p->waitid);
+		if (ast_sched_del(sched, p->waitid) == 0)
+			dialog_unref(p,"when you delete the waitid sched, you should dec the refcount for the stored dialog ptr");
+		p->waitid = -1;
 	}
 	
 
@@ -4513,7 +4520,8 @@
 				ast_clear_flag(&p->flags[0], SIP_NEEDREINVITE);	
 				if (p->waitid) {
 					ast_log(LOG_WARNING,"About to sched_del waitid(%d) for dialog %s in sip_hangup\n", p->waitid, p->callid);
-					ast_sched_del(sched, p->waitid);
+					if (ast_sched_del(sched, p->waitid) == 0)
+						dialog_unref(p,"when you delete the waitid sched, you should dec the refcount for the stored dialog ptr");
 				}
 				
 				p->waitid = -1;
@@ -5345,7 +5353,6 @@
 				 int useglobal_nat, const int intended_method)
 {
 	struct sip_pvt *p;
-	struct sip_pvt *p2;
 
 	ast_log(LOG_NOTICE,"allocating PVT for %s\n", callid);
 	if (!(p = ao2_alloc(sizeof(*p), sip_destroy_fn, "allocate a dialog(pvt) struct")))
@@ -5471,13 +5478,6 @@
 
 	/* Add to active dialog list */
 
-	ast_log(LOG_NOTICE,"***About to Search for dialog %s\n", p->callid);
-	p2 = ao2_find(dialogs, p, OBJ_POINTER, "ao2_find in dialogs");
-	if (p2) {
-		ast_log(LOG_NOTICE, "ABOUT TO LINK %s, but it is already in the dialog table!\n", p2->callid);
-		ao2_ref(p2,-1,"ao2_find success for a check");
-	}
-	
 	ao2_link(dialogs, p, "link pvt into dialogs table");
 	ast_log(LOG_NOTICE,"***Just linked %s into dialogs table\n", p->callid);
 	
@@ -13889,6 +13889,7 @@
 
 	ast_log(LOG_NOTICE,"SCHED:  sip_reinvite_retry called---\n");
 	ast_set_flag(&p->flags[0], SIP_NEEDREINVITE);	
+	dialog_unref(p,"unref the dialog ptr from sip_reinvite_retry, because it held a dialog ptr");
 	p->waitid = -1;
 	return 0;
 }
@@ -14196,7 +14197,7 @@
 				/* Reset the flag after a while 
 				 */
 				int wait = 3 + ast_random() % 5;
-				p->waitid = ast_sched_add(sched, wait, sip_reinvite_retry, p); 
+				p->waitid = ast_sched_add(sched, wait, sip_reinvite_retry, dialog_ref(p,"passing dialog ptr into sched structure based on waitid.")); 
 				ast_log(LOG_WARNING,"just did sched_add waitid(%d) for sip_reinvite_retry for dialog %s in handle_response_invite\n", p->waitid, p->callid);
 				ast_debug(2, "Reinvite race. Waiting %d secs before retry\n", wait);
 			}
@@ -20096,6 +20097,8 @@
 	struct sip_pvt *p;
 	struct ast_context *con;
 	struct ao2_iterator i;
+
+	ast_log(LOG_NOTICE,"Unload 1\n");
 	
 	/* First, take us out of the channel type list */
 	ast_channel_unregister(&sip_tech);
@@ -20106,24 +20109,30 @@
 	ast_custom_function_unregister(&sip_header_function);
 	ast_custom_function_unregister(&checksipdomain_function);
 
+	ast_log(LOG_NOTICE,"Unload 2\n");
 	/* Unregister dial plan applications */
 	ast_unregister_application(app_dtmfmode);
 	ast_unregister_application(app_sipaddheader);
 
+	ast_log(LOG_NOTICE,"Unload 3\n");
 	/* Unregister CLI commands */
 	ast_cli_unregister_multiple(cli_sip, sizeof(cli_sip) / sizeof(struct ast_cli_entry));
 
+	ast_log(LOG_NOTICE,"Unload 4\n");
 	/* Disconnect from the RTP subsystem */
 	ast_rtp_proto_unregister(&sip_rtp);
 
+	ast_log(LOG_NOTICE,"Unload 5\n");
 	/* Disconnect from UDPTL */
 	ast_udptl_proto_unregister(&sip_udptl);
 
+	ast_log(LOG_NOTICE,"Unload 6\n");
 	/* Unregister AMI actions */
 	ast_manager_unregister("SIPpeers");
 	ast_manager_unregister("SIPshowpeer");
 	ast_manager_unregister("SIPshowregistry");
 
+	ast_log(LOG_NOTICE,"Unload 7\n");
 	dialoglist_lock();
 	/* Hangup all dialogs if they have an owner */
 	i = ao2_iterator_init(dialogs, 0);
@@ -20134,6 +20143,7 @@
 	}
 	dialoglist_unlock();
 
+	ast_log(LOG_NOTICE,"Unload 8\n");
 	ast_mutex_lock(&monlock);
 	if (monitor_thread && (monitor_thread != AST_PTHREADT_STOP) && (monitor_thread != AST_PTHREADT_NULL)) {
 		pthread_cancel(monitor_thread);
@@ -20142,6 +20152,7 @@
 	}
 	monitor_thread = AST_PTHREADT_STOP;
 	ast_mutex_unlock(&monlock);
+	ast_log(LOG_NOTICE,"Unload 9\n");
 
 	dialoglist_lock();
 	/* Destroy all the dialogs and free their memory */
@@ -20152,19 +20163,23 @@
 	}
 	dialoglist_unlock();
 
+	ast_log(LOG_NOTICE,"Unload 10\n");
 	/* Free memory for local network address mask */
 	ast_free_ha(localaddr);
 
 	clear_realm_authentication(authl);
 
+	ast_log(LOG_NOTICE,"Unload 11\n");
 	ASTOBJ_CONTAINER_DESTROYALL(&regl, sip_registry_destroy);
 	ASTOBJ_CONTAINER_DESTROY(&regl);
 
+	ast_log(LOG_NOTICE,"Unload 12\n");
 	ao2_ref(peers, -1, "unref the peers table");
 	ao2_ref(peers_by_ip, -1, "unref the peers_by_ip table");
 	ao2_ref(users, -1, "unref the users table");
 	ao2_ref(dialogs, -1, "unref the dialogs table");
 
+	ast_log(LOG_NOTICE,"Unload 13\n");
 	clear_sip_domains();
 	close(sipsock);
 	sched_context_destroy(sched);




More information about the asterisk-commits mailing list