[svn-commits] murf: branch murf/bug11210 r102809 - in /team/murf/bug11210: channels/ includ...

SVN commits to the Digium repositories svn-commits at lists.digium.com
Thu Feb 7 11:04:39 CST 2008


Author: murf
Date: Thu Feb  7 11:04:38 2008
New Revision: 102809

URL: http://svn.digium.com/view/asterisk?view=rev&rev=102809
Log:
another checkpoint commit. I'm trying to perfectly balance the refcounts, making sure ALL allocated objects get destroyed (at the right time). I've still got one user struct that isn't being destroyed... all the peers are destroyed properly; this fix added another unref to the dialog_needdestroy routine, to allow dialogs that die this way to 'move on', their spirits freed to blend back into the Great Memory Pool, so to speak. I was worried about dialogs with 2-4 destructors being called, but I found to my relief that the same number of allocations have been called; I'm not surprised that memory is frequently re-used for the same sized object. <philosophic statement comparing memory to string theory removed>.

Modified:
    team/murf/bug11210/channels/chan_sip.c
    team/murf/bug11210/include/asterisk/sched.h
    team/murf/bug11210/main/astobj2.c
    team/murf/bug11210/tests/test_dlinklists.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=102809&r1=102808&r2=102809
==============================================================================
--- team/murf/bug11210/channels/chan_sip.c (original)
+++ team/murf/bug11210/channels/chan_sip.c Thu Feb  7 11:04:38 2008
@@ -2423,6 +2423,7 @@
 	AST_SCHED_DEL_UNREF(sched, dialog->autokillid, dialog_unref(dialog, "when you delete the autokillid sched, you should dec the refcount for the stored dialog ptr"));
 
 	dialog_unref(dialog, "Let's unbump the count in the unlink so the poor pvt can disappear if it is time");
+	dialog_unref(dialog, "One final decrement to counter the creation of the dialog and allow its destruction");
 	return NULL;
 }
 
@@ -2932,6 +2933,8 @@
 		if (cur == pkt) {
 			UNLINK(cur, pkt->owner->packets, prev);
 			sip_pvt_unlock(pkt->owner);
+			if (pkt->owner)
+				pkt->owner = dialog_unref(pkt->owner,"pkt is being freed, its dialog ref is dead now");
 			ast_free(pkt);
 			return 0;
 		}
@@ -3055,7 +3058,7 @@
 		append_history(p, "AutoDestroy", "%s", p->callid);
 		ast_debug(3, "Auto destroying SIP dialog '%s'\n", p->callid);
 		dialog_unlink_all(p, TRUE, TRUE); /* once it's unlinked and unrefd everywhere, it'll be freed automagically */
-		dialog_unref(p, "unref dialog-- no other matching conditions"); /* theoretically, this should be the last ref to this dialog */
+		/* dialog_unref(p, "unref dialog-- no other matching conditions"); -- unlink all now should finish off the dialog's references and free it. */
 		/* sip_destroy(p); */		/* Go ahead and destroy dialog. All attempts to recover is done */
 		/* sip_destroy also absorbs the reference */
 	}
@@ -3135,7 +3138,7 @@
 			}
 			AST_SCHED_DEL(sched, cur->retransid);
 			UNLINK(cur, p->packets, prev);
-			dialog_unref(cur->owner, "unref dialog from sip ack");
+			dialog_unref(cur->owner, "unref pkt cur->owner dialog from sip ack before freeing pkt");
 			ast_free(cur);
 			break;
 		}
@@ -12319,6 +12322,8 @@
 	   - if that's the case, wait with destruction */
 	if (dialog->needdestroy && !dialog->packets && !dialog->owner) {
 		sip_pvt_unlock(dialog);
+		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 */
 		return CMP_MATCH;
 	}
 	sip_pvt_unlock(dialog);
@@ -18359,7 +18364,7 @@
 		ast_debug(1, "Failed to grab owner channel lock, trying again. (SIP call %s)\n", p->callid);
 		sip_pvt_unlock(p);
 		if (lockretry != 1)
-			ao2_t_ref(p, -1, "release p inside lockretry loop"); /* we'll look for it again, but p is dead now */
+			ao2_t_ref(p, -1, "release p (from find_call) inside lockretry loop"); /* we'll look for it again, but p is dead now */
 		ast_mutex_unlock(&netlock);
 		/* Sleep for a very short amount of time */
 		usleep(1);
@@ -18377,7 +18382,7 @@
 			transmit_response(p, "503 Server error", req);	/* We must respond according to RFC 3261 sec 12.2 */
 		/* XXX We could add retry-after to make sure they come back */
 		append_history(p, "LockFail", "Owner lock failed, transaction failed.");
-		ao2_t_ref(p, -1, "release p at end of lockretry"); /* p is gone after the return */
+		ao2_t_ref(p, -1, "release p (from find_call) at end of lockretry"); /* p is gone after the return */
 		return 1;
 	}
 
@@ -18394,7 +18399,7 @@
 		ast_channel_unlock(p->owner);
 	sip_pvt_unlock(p);
 	ast_mutex_unlock(&netlock);
-	ao2_t_ref(p, -1, "throw away dialog ptr at end of routine"); /* p is gone after the return */
+	ao2_t_ref(p, -1, "throw away dialog ptr from find_call at end of routine"); /* p is gone after the return */
 	return 1;
 }
 

Modified: team/murf/bug11210/include/asterisk/sched.h
URL: http://svn.digium.com/view/asterisk/team/murf/bug11210/include/asterisk/sched.h?view=diff&rev=102809&r1=102808&r2=102809
==============================================================================
--- team/murf/bug11210/include/asterisk/sched.h (original)
+++ team/murf/bug11210/include/asterisk/sched.h Thu Feb  7 11:04:38 2008
@@ -43,8 +43,6 @@
 			usleep(1); \
 		if (_count == 10) \
 			ast_log(LOG_WARNING, "Unable to cancel schedule ID %d.  This is probably a bug (%s: %s, line %d).\n", id, __FILE__, __PRETTY_FUNCTION__, __LINE__); \
-		if (_count && _count != 10) \
-			ast_log(LOG_WARNING, "It took %d loops to delete schedule entry %d (%s: %s, line %d).\n", _count, id,  __FILE__, __PRETTY_FUNCTION__, __LINE__); \
 		id = -1; \
 	} while (0);
 
@@ -55,8 +53,6 @@
 			usleep(1); \
 		if (_count == 10) \
 			ast_log(LOG_WARNING, "Unable to cancel schedule ID %d.  This is probably a bug (%s: %s, line %d).\n", id, __FILE__, __PRETTY_FUNCTION__, __LINE__); \
-		if (_count && _count != 10) \
-			ast_log(LOG_WARNING, "It took %d loops to delete schedule entry %d (%s: %s, line %d).\n", _count, id,  __FILE__, __PRETTY_FUNCTION__, __LINE__); \
 		if (id > -1) \
 			refcall; \
 		id = -1; \

Modified: team/murf/bug11210/main/astobj2.c
URL: http://svn.digium.com/view/asterisk/team/murf/bug11210/main/astobj2.c?view=diff&rev=102809&r1=102808&r2=102809
==============================================================================
--- team/murf/bug11210/main/astobj2.c (original)
+++ team/murf/bug11210/main/astobj2.c Thu Feb  7 11:04:38 2008
@@ -333,6 +333,9 @@
 
 /* internal callback to destroy a container. */
 static void container_destruct(void *c);
+
+/* internal callback to destroy a container. */
+static void container_destruct_debug(void *c);
 
 /* each bucket in the container is a tailq. */
 AST_LIST_HEAD_NOLOCK(bucket, bucket_list);
@@ -396,7 +399,7 @@
 	/* XXX maybe consistency check on arguments ? */
 	/* compute the container size */
 	size_t container_size = sizeof(struct ao2_container) + n_buckets * sizeof(struct bucket);
-	struct ao2_container *c = ao2_alloc_debug(container_size, container_destruct, tag, file, line, funcname);
+	struct ao2_container *c = ao2_alloc_debug(container_size, container_destruct_debug, tag, file, line, funcname);
 	if (!c)
 		return NULL;
 	
@@ -915,11 +918,28 @@
 	return 0;
 }
 	
+static int cd_cb_debug(void *obj, void *arg, int flag)
+{
+	ao2_ref_debug(obj, -1, "deref object via container destroy",  __FILE__, __LINE__, __PRETTY_FUNCTION__);
+	return 0;
+}
+	
 static void container_destruct(void *_c)
 {
 	struct ao2_container *c = _c;
 
 	ao2_t_callback(c, OBJ_UNLINK, cd_cb, NULL, "container_destruct called");
+
+#ifdef AO2_DEBUG
+	ast_atomic_fetchadd_int(&ao2.total_containers, -1);
+#endif
+}
+
+static void container_destruct_debug(void *_c)
+{
+	struct ao2_container *c = _c;
+
+	ao2_t_callback(c, OBJ_UNLINK, cd_cb_debug, NULL, "container_destruct_debug called");
 
 #ifdef AO2_DEBUG
 	ast_atomic_fetchadd_int(&ao2.total_containers, -1);

Modified: team/murf/bug11210/tests/test_dlinklists.c
URL: http://svn.digium.com/view/asterisk/team/murf/bug11210/tests/test_dlinklists.c?view=diff&rev=102809&r1=102808&r2=102809
==============================================================================
--- team/murf/bug11210/tests/test_dlinklists.c (original)
+++ team/murf/bug11210/tests/test_dlinklists.c Thu Feb  7 11:04:38 2008
@@ -324,7 +324,7 @@
 	c = make_test1("C");
 	d = make_test1("D");
 
-	ast_log(LOG_NOTICE,"Test:\n  AST_DLLIST_MOVE_CURRENT_BACKWARDS\n  AST_DLLIST_INSERT_BEFORE_CURRENT_BACKWARDS\n");
+	ast_log(LOG_NOTICE,"Test: AST_DLLIST_MOVE_CURRENT_BACKWARDS and AST_DLLIST_INSERT_BEFORE_CURRENT_BACKWARDS\n");
 	AST_DLLIST_INSERT_HEAD(&tc->entries, a, list);
 	AST_DLLIST_INSERT_AFTER(&tc->entries, a, b, list);
 	AST_DLLIST_INSERT_AFTER(&tc->entries, b, c, list);




More information about the svn-commits mailing list