[asterisk-commits] rizzo: branch rizzo/astobj2 r47931 - /team/rizzo/astobj2/channels/chan_sip.c

asterisk-commits at lists.digium.com asterisk-commits at lists.digium.com
Wed Nov 22 10:24:54 MST 2006


Author: rizzo
Date: Wed Nov 22 11:24:53 2006
New Revision: 47931

URL: http://svn.digium.com/view/asterisk?view=rev&rev=47931
Log:
more work on tracking references for sip_registry objects.

Create a common function to break the links between a sip_pvt
and a sip_registry object, and use it in handle_response_register()
(there are some missing things in trunk).

Report a permanent failure as REG_STATE_NOAUTH aka Invalid Credentials
in 'sip show registry'


Modified:
    team/rizzo/astobj2/channels/chan_sip.c

Modified: team/rizzo/astobj2/channels/chan_sip.c
URL: http://svn.digium.com/view/asterisk/team/rizzo/astobj2/channels/chan_sip.c?view=diff&rev=47931&r1=47930&r2=47931
==============================================================================
--- team/rizzo/astobj2/channels/chan_sip.c (original)
+++ team/rizzo/astobj2/channels/chan_sip.c Wed Nov 22 11:24:53 2006
@@ -1737,7 +1737,7 @@
 	if (reg->register_pvt) {
 		/* Clear registry before destroying to ensure
 		   we don't get reentered trying to grab the registry lock */
-		reg->register_pvt->registry = NULL;
+		reg->register_pvt->registry = NULL;	/* XXX cannot be different or we would not be here! */
 		if (option_debug > 2)
 			ast_log(LOG_DEBUG, "Destroying active SIP dialog for registry %s@%s\n", reg->username, reg->hostname);
 		reg->register_pvt = sip_destroy(reg->register_pvt);
@@ -1749,9 +1749,10 @@
 	free(reg);
 }
 
-static void unref_registry(struct sip_registry *reg)
+static void *unref_registry(struct sip_registry *reg)
 {
 	ASTOBJ_UNREF(reg, sip_registry_destroy);
+	return NULL;
 }
 
 static void registry_destroy_all(void)
@@ -3229,7 +3230,7 @@
 		 */
 		if (p->registry->register_pvt == p)
 			p->registry->register_pvt = pvt_unref(p->registry->register_pvt);
-		unref_registry(p->registry);
+		p->registry = unref_registry(p->registry);
 	}
 
 	/* Unlink us from the owner if we have one */
@@ -4814,7 +4815,7 @@
 	reg->callid_valid = FALSE;
 	reg->ocseq = INITIAL_CSEQ;
 	ASTOBJ_CONTAINER_LINK(&regl, reg);	/* Add the new registry entry to the list */
-	unref_registry(reg);
+	unref_registry(reg);		/* release the reference given by ASTOBJ_INIT. The container has another reference */
 	return 0;
 }
 
@@ -7360,7 +7361,7 @@
 	case REG_STATE_TIMEOUT:
 		return "Timeout";
 	case REG_STATE_NOAUTH:
-		return "No Authentication";
+		return "Invalid Credentials";
 	default:
 		return "Unknown";
 	}
@@ -7418,11 +7419,13 @@
 		   in the single SIP manager thread. */
 		/* XXX p->registry == r so and r has 2 refs, so the unref won't take the object away */
 		p = r->register_pvt;
-		if (p->registry)
-			unref_registry(p->registry);
 		set_destroy(p);	/* XXX why do that if we are going to kill it in a moment ? */
 		/* Pretend to ACK anything just in case */
 		__sip_pretend_ack(p); /* XXX we need p locked, not sure we have */
+
+		/* decouple the two objects */
+		if (p->registry)
+			p->registry = unref_registry(p->registry);
 		r->register_pvt = pvt_unref(r->register_pvt);	/* reference goes away */
 	}
 
@@ -7459,7 +7462,14 @@
 		ast_log(LOG_NOTICE, "Strange, trying to register %s@%s when registration already pending\n", r->username, r->hostname);
 		return 0;
 	}
-
+	if (r->timeout > -1) {
+		ast_log(LOG_WARNING, "transmit_register should not have a pending timeout -- %s@%s, event %d\n",
+		r->username, r->hostname, r->timeout);
+		ast_sched_del(sched, r->timeout);
+		r->timeout = -1;
+	}
+
+	r->regattempts++;
 	if (r->register_pvt) {	/* We have a registration */
 		if (!auth) {
 			ast_log(LOG_WARNING, "Already have a REGISTER going on to %s@%s?? \n", r->username, r->hostname);
@@ -7476,27 +7486,20 @@
 			r->callid_valid = TRUE;
 		}
 		/* Allocate SIP dialog for registration */
-		if (!(p = sip_alloc( r->callid, NULL, 0, SIP_REGISTER))) {
+		if (!(p = sip_alloc(r->callid, NULL, 0, SIP_REGISTER))) {
 			ast_log(LOG_WARNING, "Unable to allocate registration transaction (memory or socket error)\n");
-			return 0;
+		} else if (create_addr(p, r->hostname)) {	/* lookup the address */
+			/* Address lookup error, hopefully just a DNS problem, so retry later */
+			ast_log(LOG_WARNING, "Probably a DNS error for registration to %s@%s, trying REGISTER again after %d seconds)\n",
+					r->username, r->hostname, global_reg_timeout);
+			p = sip_destroy(p);	/* and the reference goes */
+		}
+		if (p == NULL) {
+			r->timeout = ast_sched_add(sched, global_reg_timeout*1000, sip_reg_timeout, r);
+			return 0;	/* non fatal error */
 		}
 		if (!ast_test_flag(&p->flags[0], SIP_NO_HISTORY))
 			append_history(p, "RegistryInit", "Account: %s@%s", r->username, r->hostname);
-		/* Find address to hostname */
-		if (create_addr(p, r->hostname)) {
-			/* we have what we hope is a temporary network error,
-			 * probably DNS.  We need to reschedule a registration try */
-			sip_destroy(p);	/* and the reference goes */
-			if (r->timeout > -1) {
-				ast_log(LOG_WARNING, "Still have a registration timeout for %s@%s (create_addr() error), event %d\n", r->username, r->hostname, r->timeout);
-				ast_sched_del(sched, r->timeout);
-			} else {
-				ast_log(LOG_WARNING, "Probably a DNS error for registration to %s@%s, trying REGISTER again (after %d seconds)\n", r->username, r->hostname, global_reg_timeout);
-			}
-			r->timeout = ast_sched_add(sched, global_reg_timeout*1000, sip_reg_timeout, r);
-			r->regattempts++;
-			return 0;	/* non fatal error */
-		}
 		/* Copy back Call-ID in case create_addr changed it */
 		ast_string_field_set(r, callid, p->callid);
 		if (r->portno)
@@ -7536,14 +7539,14 @@
 		build_contact(p);
 	}
 
-	/* set up a timeout */
-		if (r->timeout > -1) {
-			ast_log(LOG_WARNING, "Still have a registration timeout, #%d - deleting it\n", r->timeout);
-			ast_sched_del(sched, r->timeout);
-		}
-		r->timeout = ast_sched_add(sched, global_reg_timeout * 1000, sip_reg_timeout, r);
-		if (option_debug)
-			ast_log(LOG_DEBUG, "Scheduled a registration timeout for %s id  #%d \n", r->hostname, r->timeout);
+	/* set up a timeout.
+	 * In principle, the send_request() at the end is reliable, but there is
+	 * no provision at the moment to notify us if the transmission fails,
+	 * so we need to set this extra timeout.
+	 */
+	r->timeout = ast_sched_add(sched, global_reg_timeout * 1000, sip_reg_timeout, r);
+	if (option_debug)
+		ast_log(LOG_DEBUG, "Scheduled a registration timeout for %s id  #%d \n", r->hostname, r->timeout);
 
 	if (strchr(r->username, '@')) {
 		snprintf(from, sizeof(from), "<sip:%s>;tag=%s", r->username, p->tag);
@@ -7582,9 +7585,12 @@
 	add_header(&req, "User-Agent", global_useragent);
 
 	
-	if (auth) 	/* Add auth header */
+	if (auth) {	/* Add auth header */
 		add_header(&req, authheader, auth);
-	else if (!ast_strlen_zero(r->nonce)) {
+		ast_verbose("REGISTER attempt %d to %s@%s, %s: %s\n",
+			r->regattempts, r->username, r->hostname,
+			authheader, auth);
+	} else if (!ast_strlen_zero(r->nonce)) {
 		char digest[1024];
 
 		/* We have auth data to reuse, build a digest header! */
@@ -7598,9 +7604,12 @@
 		p->noncecount = r->noncecount++;
 
 		memset(digest,0,sizeof(digest));
-		if(!build_reply_digest(p, SIP_REGISTER, digest, sizeof(digest)))
+		if(!build_reply_digest(p, SIP_REGISTER, digest, sizeof(digest))) {
 			add_header(&req, "Authorization", digest);
-		else
+			ast_verbose("REUSE REGISTER attempt %d to %s@%s, %s: %s\n",
+				r->regattempts, r->username, r->hostname,
+				"Authorization", digest);
+		} else
 			ast_log(LOG_NOTICE, "No authorization available for authentication of registration to %s@%s\n", r->username, r->hostname);
 	
 	}
@@ -7614,10 +7623,11 @@
 	initialize_initreq(p, &req);
 	if (sip_debug_test_pvt(p))
 		ast_verbose("REGISTER %d headers, %d lines\n", p->initreq.headers, p->initreq.lines);
+	if (option_debug > 3)
+		ast_verbose("REGISTER %d %s@%s goes from %s to %s\n",
+			r->regattempts, r->username, r->hostname, regstate2str(r->regstate),
+			regstate2str(auth ? REG_STATE_AUTHSENT : REG_STATE_REGSENT) );
 	r->regstate = auth ? REG_STATE_AUTHSENT : REG_STATE_REGSENT;
-	r->regattempts++;	/* Another attempt */
-	if (option_debug > 3)
-		ast_verbose("REGISTER attempt %d to %s@%s\n", r->regattempts, r->username, r->hostname);
 	ret = send_request(p, &req, XMIT_CRITICAL, p->ocseq);
 	pvt_unref(p);
 	return ret;
@@ -12303,6 +12313,20 @@
 	}
 }
 
+/*! \brief decouple the sip_pvt and the sip_registry, and mark the pvt with set_destroy.
+ * The objects are still in their containers so they don't go away,
+ * or we should be careful with the release order (by keeping a
+ * reference and releasing afterwards).
+ */
+static void reg_pvt_unlink(struct sip_pvt *p)
+{
+	struct sip_registry *r = p->registry;
+
+	r->register_pvt = pvt_unref(r->register_pvt);
+	p->registry = unref_registry(r);
+	set_destroy(p);
+}
+
 /*! \brief Handle responses on REGISTER to services.
  * In principle, we should have p->registry != NULL,
  * and p->registry->register_pvt == p, otherwise there is
@@ -12336,6 +12360,10 @@
 		r->timeout = -1;
 		/* XXX in the future, remember the reference to r */
 	}
+
+	if (option_debug > 3)
+		ast_verbose("REGISTER %d %s@%s response %d state %s\n",
+			r->regattempts, r->username, r->hostname, resp, regstate2str(r->regstate));
 
 	switch (resp) {
 	case 401:	/* Unauthorized */
@@ -12367,8 +12395,7 @@
 		}
 		r->regstate = REG_STATE_NOAUTH;
 		ast_log(LOG_NOTICE, "Failed to authenticate on REGISTER to '%s@%s' (Tries %d)\n", r->username, r->hostname, p->authtries);
-		r->register_pvt = pvt_unref(r->register_pvt);	/* don't care anymore */
-		set_destroy(p);
+		reg_pvt_unlink(p);
 		break;
 
 	case 403:	/* Forbidden */
@@ -12377,15 +12404,14 @@
 			ast_log(LOG_WARNING, "Forbidden - wrong password on authentication for REGISTER for '%s' to '%s'\n", r->username, r->hostname);
 		else
 			ast_log(LOG_WARNING, "Got 404 Not found on SIP register to service %s@%s, giving up\n", r->username, r->hostname);
-		set_destroy(p);
-		r->register_pvt = pvt_unref(r->register_pvt);
+		r->regstate = REG_STATE_NOAUTH;
+		reg_pvt_unlink(p);
 		break;
 
 	case 423:	/* Interval too brief */
 		r->expiry = atoi(get_header(req, "Min-Expires"));
 		ast_log(LOG_WARNING, "Got 423 Interval too brief for service %s@%s, minimum is %d seconds\n", r->username, r->hostname, r->expiry);
-		set_destroy(p);
-		r->register_pvt = pvt_unref(r->register_pvt);
+		reg_pvt_unlink(p);
 		if (r->expiry > max_expiry) {
 			ast_log(LOG_WARNING, "Required expiration time from %s@%s is too high, giving up\n", r->username, r->hostname);
 			r->expiry = default_expiry;
@@ -12399,8 +12425,7 @@
 
 	case 479:	/* SER: Not able to process the URI - address is wrong in register*/
 		ast_log(LOG_WARNING, "Got error 479 on register to %s@%s, giving up (check config)\n", r->username, r->hostname);
-		set_destroy(p);
-		r->register_pvt = pvt_unref(r->register_pvt);
+		reg_pvt_unlink(p);
 		break;
 
 	case 200:	/* 200 OK */
@@ -12411,20 +12436,13 @@
 		if (option_debug)
 			ast_log(LOG_DEBUG, "Registration successful\n");
 
-		/* unlink the dialog from the registry */
-		r->register_pvt = pvt_unref(r->register_pvt);
-		p->registry = NULL;
-
-		/* Let this one hang around until we have all the responses (XXX what for ?)*/
-		sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT);
-		/* set_destroy(p);	*/
+		reg_pvt_unlink(p);
 
 		/* set us up for re-registering */
 		/* according to section 6.13 of RFC, contact headers override
 		   expires headers, so check those first */
 		expires = 0;
 
-		/* XXX todo: try to save the extra call */
 		if (!ast_strlen_zero(get_header(req, "Contact"))) {
 			const char *contact = NULL;
 			const char *tmptmp = NULL;
@@ -12464,7 +12482,6 @@
 
 		/* Schedule re-registration before we expire */
 		r->timeout=ast_sched_add(sched, expires_ms, sip_reregister, r); 
-		unref_registry(r);
 	}
 	return 1;
 }



More information about the asterisk-commits mailing list