[svn-commits] rizzo: branch rizzo/astobj2 r47925 -
/team/rizzo/astobj2/channels/chan_sip.c
svn-commits at lists.digium.com
svn-commits at lists.digium.com
Wed Nov 22 04:55:46 MST 2006
Author: rizzo
Date: Wed Nov 22 05:55:45 2006
New Revision: 47925
URL: http://svn.digium.com/view/asterisk?view=rev&rev=47925
Log:
Start cleaning up the handling of outbound registrations.
In detail:
- Add some comment on the states sipregistrystate.
This documentation is still largely incomplete;
- remove the 'expire' fields from struct sip_registry - now there
is only one pending event, "timeout", which is used for both
refresh and retransmissions. Much simpler to understand.
- set up a timeout also when retransmitting a REGISTER with auth set.
Previously this was not done.
- cancel a timeout early in handle_response_register.
I think this is always the thing to do. Not sure how does
a registration restart on a permanent failure (or if we want it
to happen at all).
- add a missing r->register_pvt = pvt_unref(r->register_pvt);
in case "Failed to authenticate on REGISTER"
I still fail to understand why we are keeping the dialog alive
(sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT) instead of set_destroy(p) )
after receiving the 200 OK to the registration .
The handling of references to the sip_registry is still incorrect.
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=47925&r1=47924&r2=47925
==============================================================================
--- team/rizzo/astobj2/channels/chan_sip.c (original)
+++ team/rizzo/astobj2/channels/chan_sip.c Wed Nov 22 05:55:45 2006
@@ -1191,17 +1191,20 @@
/*! \brief States for outbound registrations (with register= lines in sip.conf */
enum sipregistrystate {
REG_STATE_UNREGISTERED = 0, /*!< We are not registred */
- /* Initial state. We should have r->expire > -1 to indicate
- * a scheduled transmission of the initial registration request.
- * When the timeout fires, we transmit the request and move to ...
+ /* Initial state. We should have a timeout scheduled for the initial
+ * (or next) registration transmission, calling sip_reregister
*/
REG_STATE_REGSENT, /*!< Registration request sent */
+ /* sent initial request, waiting for an ack or a timeout to
+ * retransmit the initial request.
+ */
REG_STATE_AUTHSENT, /*!< We have tried to authenticate */
REG_STATE_REGISTERED, /*!< Registred and done */
REG_STATE_REJECTED, /*!< Registration rejected */
REG_STATE_TIMEOUT, /*!< Registration timed out */
REG_STATE_NOAUTH, /*!< We have no accepted credentials */
REG_STATE_FAILED, /*!< Registration failed after several tries */
+ /* fatal - no chance to proceed */
};
/*!
@@ -1233,10 +1236,9 @@
AST_STRING_FIELD(random);
);
int portno; /*!< Optional port override */
- int expire; /*!< Sched ID of expiration */
int expiry; /*!< Value to use for the Expires header */
int regattempts; /*!< Number of attempts (since the last success) */
- int timeout; /*!< sched id of sip_reg_timeout */
+ int timeout; /*!< sched id of next event */
int refresh; /*!< How often to refresh */
struct sip_pvt *register_pvt; /*!< create a sip_pvt structure for each outbound "registration dialog" in progress */
enum sipregistrystate regstate; /*!< Registration state (see above) */
@@ -1726,8 +1728,6 @@
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);
}
- if (reg->expire > -1)
- ast_sched_del(sched, reg->expire);
if (reg->timeout > -1)
ast_sched_del(sched, reg->timeout);
ast_string_field_free_pools(reg);
@@ -3231,7 +3231,7 @@
if (n != 1) {
ast_verbose("__sip_destroy on object with %d refs\n", n);
ao2_bt();
- *(char *)0 = 1;/* crash */
+ // *(char *)0 = 1;/* crash */
}
}
pvt_unref(p); /* automatically calls pvt_destructor on ao2*/
@@ -4790,7 +4790,6 @@
if (secret)
ast_string_field_set(reg, secret, secret);
reg->regstate = REG_STATE_UNREGISTERED;
- reg->expire = -1;
reg->expiry = default_expiry;
reg->timeout = -1;
reg->refresh = default_expiry;
@@ -7367,6 +7366,7 @@
if (!r)
return 0;
+ /* XXX why do we want to keep a dialog here ??? */
pvt = r->register_pvt; /* XXX do we have a lock on it ? */
if (pvt && !ast_test_flag(&pvt->flags[0], SIP_NO_HISTORY))
append_history(pvt, "RegistryRenew", "Account: %s@%s", r->username, r->hostname);
@@ -7375,7 +7375,7 @@
if (sipdebug)
ast_log(LOG_NOTICE, " -- Re-registration for %s@%s\n", r->username, r->hostname);
- r->expire = -1;
+ r->timeout = -1;
transmit_register(r, NULL, NULL);
unref_registry(r);
return 0;
@@ -7531,7 +7531,6 @@
}
/* set up a timeout */
- if (auth == NULL) {
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);
@@ -7539,7 +7538,6 @@
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);
@@ -12326,7 +12324,7 @@
/*! \brief Handle responses on REGISTER to services.
* In principle, we should have p->registry != NULL,
* and p->registry->register_pvt == p, otherwise there is
- * something bogus
+ * something bogus.
*/
static int handle_response_register(struct sip_pvt *p, int resp, char *rest, struct sip_request *req, int seqno)
{
@@ -12342,12 +12340,31 @@
return 1;
}
+ /*
+ * Not sure if it is the right thing to do, but probably it is ok
+ * to cancel the timeout here, since we have already matched the
+ * packet with the sip_registry, and so this is a valid entry
+ * (how about sequence numbers ?).
+ */
+ if (r->timeout == -1) {
+ ast_log(LOG_NOTICE, "Got %d without pending timeout for registry '%s@%s'\n",
+ resp, r->username, r->hostname);
+ } else {
+ ast_sched_del(sched, r->timeout);
+ r->timeout = -1;
+ /* XXX in the future, remember the reference to r */
+ }
+
switch (resp) {
case 401: /* Unauthorized */
case 407: /* Proxy auth */
- /* retransmit with proper auth credentials */
+ /* Possibly retransmit with proper auth credentials.
+ * This is the only case where it makes sense to keep the old
+ * dialog. In all other cases, we don't need it anymore.
+ */
if (p->authtries == MAX_AUTHTRIES || do_register_auth(p, req, resp)) {
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);
}
break;
@@ -12361,8 +12378,6 @@
/* XXX how is this useful given we have no pending event anymore ??? */
if (global_regattempts_max)
r->regattempts = global_regattempts_max+1;
- ast_sched_del(sched, r->timeout);
- r->timeout = -1;
set_destroy(p);
r->register_pvt = pvt_unref(r->register_pvt);
break;
@@ -12370,8 +12385,6 @@
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);
- ast_sched_del(sched, r->timeout);
- r->timeout = -1;
set_destroy(p);
r->register_pvt = pvt_unref(r->register_pvt);
if (r->expiry > max_expiry) {
@@ -12392,9 +12405,8 @@
r->regattempts = global_regattempts_max+1;
set_destroy(p);
r->register_pvt = pvt_unref(r->register_pvt);
- ast_sched_del(sched, r->timeout);
- r->timeout = -1;
break;
+
case 200: /* 200 OK */
r->regstate = REG_STATE_REGISTERED;
r->regtime = time(NULL); /* Reset time of last succesful registration */
@@ -12402,23 +12414,16 @@
r->regattempts = 0;
if (option_debug)
ast_log(LOG_DEBUG, "Registration successful\n");
- if (r->timeout > -1) {
- if (option_debug)
- ast_log(LOG_DEBUG, "Cancelling timeout %d\n", r->timeout);
- ast_sched_del(sched, r->timeout);
- }
- r->timeout=-1;
+
+ /* 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 */
+
+ /* Let this one hang around until we have all the responses (XXX what for ?)*/
sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT);
/* set_destroy(p); */
/* set us up for re-registering */
- /* figure out how long we got registered for */
- if (r->expire > -1)
- ast_sched_del(sched, r->expire);
- r->expire = -1;
/* according to section 6.13 of RFC, contact headers override
expires headers, so check those first */
expires = 0;
@@ -12462,7 +12467,7 @@
r->refresh= (int) expires_ms / 1000;
/* Schedule re-registration before we expire */
- r->expire=ast_sched_add(sched, expires_ms, sip_reregister, r);
+ r->timeout=ast_sched_add(sched, expires_ms, sip_reregister, r);
unref_registry(r);
}
return 1;
@@ -17403,14 +17408,14 @@
ms = regspacing;
ASTOBJ_CONTAINER_TRAVERSE(®l, 1, do {
ASTOBJ_WRLOCK(iterator);
- if (iterator->expire > -1)
- ast_sched_del(sched, iterator->expire);
+ if (iterator->timeout > -1)
+ ast_sched_del(sched, iterator->timeout);
ms += regspacing;
/* grab a reference to make sure objects do not go away when
* the timeout is canceled.
*/
ASTOBJ_REF(iterator);
- iterator->expire = ast_sched_add(sched, ms, sip_reregister, iterator);
+ iterator->timeout = ast_sched_add(sched, ms, sip_reregister, iterator);
ASTOBJ_UNLOCK(iterator);
} while (0)
);
More information about the svn-commits
mailing list