Improve object destruction (bug #3286)

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@4747 65c4cc65-6c06-0410-ace0-fbb531ad65f3

channels/chan_sip.c

@@ -1111,66 +1111,44 @@ static void update_peer(struct sip_peer *p, int expiry)
 		realtime_update_peer(p->name, &p->addr, p->username, expiry);
 }
 
-static struct sip_peer *build_peer(const char *name, struct ast_variable *v, int temponly);
+static struct sip_peer *build_peer(const char *name, struct ast_variable *v, int realtime);
 
 static struct sip_peer *realtime_peer(const char *peername, struct sockaddr_in *sin)
 {
-	struct ast_variable *var, *tmp=NULL;
-	char iabuf[80];
 	struct sip_peer *peer=NULL;
-	time_t nowtime, regseconds;
-	int dynamic=0;
+	struct ast_variable *var;
+	struct ast_variable *tmp;
 
-	if (sin)
-		ast_inet_ntoa(iabuf, sizeof(iabuf), sin->sin_addr);
 	if (peername) 
 		var = ast_load_realtime("sipfriends", "name", peername, NULL);
-	else
+	else if (sin) {
+		char iabuf[80];
+
+		ast_inet_ntoa(iabuf, sizeof(iabuf), sin->sin_addr);
 		var = ast_load_realtime("sipfriends", "ipaddr", iabuf, NULL);
-	if (var) {
-		/* Make sure it's not a user only... */
-		peer = build_peer(peername, var, 1);
-		if (peer) {
-			/* Add some finishing touches, addresses, etc */
-			ast_set_flag(peer, SIP_REALTIME);
+	} else
+		return NULL;
+
+	if (!var)
+		return NULL;
+
 	tmp = var;
 	while(tmp) {
-				if (!strcasecmp(tmp->name, "type")) {
-					if (strcasecmp(tmp->value, "friend") &&
-						strcasecmp(tmp->value, "peer")) {
-						/* Whoops, we weren't supposed to exist! */
-						sip_destroy_peer(peer);
-						peer = NULL;
-						break;
-					} 
-				} else if (!strcasecmp(tmp->name, "regseconds")) {
-					if (sscanf(tmp->value, "%li", &regseconds) != 1)
-						regseconds = 0;
-				} else if (!strcasecmp(tmp->name, "ipaddr")) {
-					inet_aton(tmp->value, &(peer->addr.sin_addr));
-				} else if (!strcasecmp(tmp->name, "port")) {
-					peer->addr.sin_port = htons(atoi(tmp->value));
-				} else if (!strcasecmp(tmp->name, "host")) {
-					if (!strcasecmp(tmp->value, "dynamic"))
-						dynamic = 1;
+		if (strcasecmp(tmp->name, "type"))
+			continue;
+
+		if (!strcasecmp(tmp->value, "user")) {
+			ast_destroy_realtime(var);
+			return NULL;
 		}
+
 		tmp = tmp->next;
 	}
-			if (peer && dynamic) {
-				time(&nowtime);
-				if ((nowtime - regseconds) > 0) {
-					memset(&peer->addr, 0, sizeof(peer->addr));
-					if (option_debug)
-						ast_log(LOG_DEBUG, "Bah, we're expired (%ld/%ld/%ld)!\n", nowtime - regseconds, regseconds, nowtime);
-				}
-			}
-		}
+
+	peer = build_peer(peername, var, 1);
+	if (peer)
+		ast_set_flag(peer, SIP_REALTIME);
 	ast_destroy_realtime(var);
-	}
-	if (peer) {
-		/* Destroy, so when our caller unrefs, it will disappear */
-		ASTOBJ_DESTROY(peer, sip_destroy_peer);
-	}
 	return peer;
 }
 
@@ -1214,43 +1192,39 @@ static void sip_destroy_user(struct sip_user *user)
 	free(user);
 }
 
-static struct sip_user *build_user(const char *name, struct ast_variable *v);
+static struct sip_user *build_user(const char *name, struct ast_variable *v, int realtime);
 static struct sip_user *realtime_user(const char *username)
 {
 	struct ast_variable *var;
 	struct ast_variable *tmp;
 	struct sip_user *user = NULL;
+
 	var = ast_load_realtime("sipfriends", "name", username, NULL);
-	if (var) {
-		/* Make sure it's not a user only... */
-		user = build_user(username, var);
+
+	if (!var)
+		return NULL;
+
+	tmp = var;
+	while (tmp) {
+		if (strcasecmp(tmp->name, "type"))
+			continue;
+
+		if (!strcasecmp(tmp->value, "peer")) {
+			ast_destroy_realtime(var);
+			return NULL;
+		}
+		tmp = tmp->next;
+	}
+
+	user = build_user(username, var, 1);
 	if (user) {
 		/* Move counter from s to r... */
 		suserobjs--;
 		ruserobjs++;
 		/* Add some finishing touches, addresses, etc */
 		ast_set_flag(user, SIP_REALTIME);	
-			tmp = var;
-			while(tmp) {
-				if (!strcasecmp(tmp->name, "type")) {
-					if (strcasecmp(tmp->value, "friend") &&
-						strcasecmp(tmp->value, "user")) {
-						/* Whoops, we weren't supposed to exist! */
-						sip_destroy_user(user);
-						user = NULL;
-						break;
-					} 
-				}
-				tmp = tmp->next;
-			}
 	}
 	ast_destroy_realtime(var);
-	}
-	if (user) {
-		/* Reference and destroy, so when our caller unrefs, we disappear */
-		ASTOBJ_REF(user);
-		ASTOBJ_DESTROY(user, sip_destroy_user);
-	}
 	return user;
 }
 
@@ -8644,7 +8618,7 @@ static int handle_common_options(struct ast_flags *flags, struct ast_flags *mask
 }
 
 /*--- build_user: Initiate a SIP user structure from sip.conf ---*/
-static struct sip_user *build_user(const char *name, struct ast_variable *v)
+static struct sip_user *build_user(const char *name, struct ast_variable *v, int realtime)
 {
 	struct sip_user *user;
 	int format;
@@ -8778,15 +8752,16 @@ static struct sip_peer *temp_peer(char *name)
 }
 
 /*--- build_peer: Build peer from config file ---*/
-static struct sip_peer *build_peer(const char *name, struct ast_variable *v, int temponly)
+static struct sip_peer *build_peer(const char *name, struct ast_variable *v, int realtime)
 {
 	struct sip_peer *peer = NULL;
 	struct ast_ha *oldha = NULL;
 	int maskfound=0;
 	int obproxyfound=0;
 	int found=0;
+	time_t regseconds;
 
-	if (!temponly)
+	if (!realtime)
 		/* Note we do NOT use find_peer here, to avoid realtime recursion */
 		peer = ASTOBJ_CONTAINER_FIND_UNLINK(&peerl, name);
 
@@ -8797,7 +8772,7 @@ static struct sip_peer *build_peer(const char *name, struct ast_variable *v, int
 		peer = malloc(sizeof(struct sip_peer));
 		if (peer) {
 			memset(peer, 0, sizeof(struct sip_peer));
-			if (temponly)
+			if (realtime)
 				rpeerobjs++;
 			else
 				speerobjs++;
@@ -8838,7 +8813,12 @@ static struct sip_peer *build_peer(const char *name, struct ast_variable *v, int
 				continue;
 			}
 
-			if (!strcasecmp(v->name, "name"))
+			if (realtime && !strcasecmp(v->name, "regseconds")) {
+				if (sscanf(v->value, "%li", &regseconds) != 1)
+					regseconds = 0;
+			} else if (realtime && !strcasecmp(v->name, "ipaddr")) {
+				inet_aton(v->value, &(peer->addr.sin_addr));
+			} else if (realtime && !strcasecmp(v->name, "name"))
 				strncpy(peer->name, v->value, sizeof(peer->name)-1);
 			else if (!strcasecmp(v->name, "secret")) 
 				strncpy(peer->secret, v->value, sizeof(peer->secret)-1);
@@ -8880,7 +8860,7 @@ static struct sip_peer *build_peer(const char *name, struct ast_variable *v, int
 					ast_clear_flag(peer, SIP_DYNAMIC);	
 					if (!obproxyfound || !strcasecmp(v->name, "outboundproxy")) {
 						if (ast_get_ip_or_srv(&peer->addr, v->value, "_sip._udp")) {
-							ASTOBJ_DESTROY(peer, sip_destroy_peer);
+							ASTOBJ_UNREF(peer, sip_destroy_peer);
 							return NULL;
 						}
 					}
@@ -8893,7 +8873,7 @@ static struct sip_peer *build_peer(const char *name, struct ast_variable *v, int
 					inet_aton("255.255.255.255", &peer->mask);
 			} else if (!strcasecmp(v->name, "defaultip")) {
 				if (ast_get_ip(&peer->defaddr, v->value)) {
-					ASTOBJ_DESTROY(peer, sip_destroy_peer);
+					ASTOBJ_UNREF(peer, sip_destroy_peer);
 					return NULL;
 				}
 			} else if (!strcasecmp(v->name, "permit") ||
@@ -8903,7 +8883,7 @@ static struct sip_peer *build_peer(const char *name, struct ast_variable *v, int
 				maskfound++;
 				inet_aton(v->value, &peer->mask);
 			} else if (!strcasecmp(v->name, "port")) {
-				if (ast_test_flag(peer, SIP_DYNAMIC))
+				if (!realtime && ast_test_flag(peer, SIP_DYNAMIC))
 					peer->defaddr.sin_port = htons(atoi(v->value));
 				else
 					peer->addr.sin_port = htons(atoi(v->value));
@@ -8955,6 +8935,16 @@ static struct sip_peer *build_peer(const char *name, struct ast_variable *v, int
 			 */
 			v=v->next;
 		}
+		if (realtime && ast_test_flag(peer, SIP_DYNAMIC)) {
+			time_t nowtime;
+
+			time(&nowtime);
+			if ((nowtime - regseconds) > 0) {
+				memset(&peer->addr, 0, sizeof(peer->addr));
+				if (option_debug)
+					ast_log(LOG_DEBUG, "Bah, we're expired (%ld/%ld/%ld)!\n", nowtime - regseconds, regseconds, nowtime);
+			}
+		}
 		ast_copy_flags(peer, &peerflags, mask.flags);
 		if (!found && ast_test_flag(peer, SIP_DYNAMIC))
 			reg_source_db(peer);
@@ -9200,7 +9190,7 @@ static int reload_config(void)
 			utype = ast_variable_retrieve(cfg, cat, "type");
 			if (utype) {
 				if (!strcasecmp(utype, "user") || !strcasecmp(utype, "friend")) {
-					user = build_user(cat, ast_variable_browse(cfg, cat));
+					user = build_user(cat, ast_variable_browse(cfg, cat), 0);
 					if (user) {
 						ASTOBJ_CONTAINER_LINK(&userl,user);
 						ASTOBJ_UNREF(user, sip_destroy_user);

include/asterisk/astobj.h

@@ -32,8 +32,7 @@ extern "C" {
 #define ASTOBJ_DEFAULT_BUCKETS	256
 #define ASTOBJ_DEFAULT_HASH		ast_strhash
 
-#define ASTOBJ_FLAG_DELME 	(1 << 0)		/* Object has been deleted, remove on last unref */
-#define ASTOBJ_FLAG_MARKED	(1 << 1)		/* Object has been marked for possible deletion */
+#define ASTOBJ_FLAG_MARKED	(1 << 0)		/* Object has been marked for future operation */
 
 #if __GNUC__ < 2 || (__GNUC__ == 2 && __GNUC_MINOR__ < 96)
 #define __builtin_expect(exp, c) (exp)
@@ -81,13 +80,17 @@ extern "C" {
 	
 #define ASTOBJ_UNREF(object,destructor) \
 	do { \
+		int newcount = 0; \
 		ASTOBJ_WRLOCK(object); \
 		if (__builtin_expect((object)->refcount, 1)) \
-			(object)->refcount--; \
+			newcount = --((object)->refcount); \
 		else \
 			ast_log(LOG_WARNING, "Unreferencing unreferenced (object)!\n"); \
 		ASTOBJ_UNLOCK(object); \
-		ASTOBJ_DESTROY(object,destructor); \
+		if (newcount == 0) { \
+			ast_mutex_destroy(&(object)->_lock); \
+			destructor((object)); \
+		} \
 		(object) = NULL; \
 	} while(0)
 
@@ -105,18 +108,6 @@ extern "C" {
 		ASTOBJ_UNLOCK(object); \
 	} while(0)
 
-#define ASTOBJ_DESTROY(object,destructor) \
-	do { \
-		if (__builtin_expect((object)->refcount, 1)) { \
-			ASTOBJ_WRLOCK(object); \
-			(object)->objflags |= ASTOBJ_FLAG_DELME; \
-			ASTOBJ_UNLOCK(object); \
-		} else { \
-			ast_mutex_destroy(&(object)->_lock); \
-			destructor((object)); \
-		} \
-	} while(0)
-	
 #define ASTOBJ_INIT(object) \
 	do { \
 		ast_mutex_init(&(object)->_lock); \
@@ -190,7 +181,6 @@ extern "C" {
 		ASTOBJ_CONTAINER_WRLOCK(container); \
 		while((iterator = (container)->head)) { \
 			(container)->head = (iterator)->next[0]; \
-			ASTOBJ_DESTROY(iterator,destructor); \
 			ASTOBJ_UNREF(iterator,destructor); \
 		} \
 		ASTOBJ_CONTAINER_UNLOCK(container); \
@@ -203,6 +193,7 @@ extern "C" {
 		ASTOBJ_CONTAINER_TRAVERSE(container, !found, do { \
 			if (!(strcasecmp(iterator->name, (namestr)))) { \
 				found = iterator; \
+				found->next[0] = NULL; \
 				ASTOBJ_CONTAINER_WRLOCK(container); \
 				if (prev) \
 					prev->next[0] = next; \
@@ -223,6 +214,7 @@ extern "C" {
 			ASTOBJ_RDLOCK(iterator); \
 			if (!(comparefunc(iterator->field, (data)))) { \
 				found = iterator; \
+				found->next[0] = NULL; \
 				ASTOBJ_CONTAINER_WRLOCK(container); \
 				if (prev) \
 					prev->next[0] = next; \
@@ -249,7 +241,7 @@ extern "C" {
 					(container)->head = next; \
 				ASTOBJ_CONTAINER_UNLOCK(container); \
 				ASTOBJ_UNLOCK(iterator); \
-				ASTOBJ_DESTROY(iterator,destructor); \
+				ASTOBJ_UNREF(iterator,destructor); \
 				continue; \
 			} \
 			ASTOBJ_UNLOCK(iterator); \