Skip to content
Snippets Groups Projects
Commit 9bf67151 authored by David Vossel's avatar David Vossel
Browse files

SIP registry ref count error 

During a sip reload, the list of sip_registry objects are
supposed to be traversed, unlinked, and destroyed, but
destruction never takes place due to a ref counting error.
This causes a memory leak when registry items are removed
from sip.conf and reloaded.  While the registries are removed
from the global list, they are not removed from the scheduler.
Because of this, SIP register attempts continue to be sent
out for the item even though it may no longer be in the .conf.

(closes issue #15295)
Reported by: amorsen

Review: https://reviewboard.asterisk.org/r/282/


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@201344 65c4cc65-6c06-0410-ace0-fbb531ad65f3
parent 940accbd
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
Showing
with 18 additions and 29 deletions
channels/chan_sip.c
+ 18
− 29
View file @ 9bf67151
......@@ -2062,12 +2062,6 @@ struct sip_peer {
* or once the previously completed registration one expires).
* The registration can be in one of many states, though at the moment
* the handling is a bit mixed.
*
* XXX \todo Reference count handling for this object has some problems with
* respect to scheduler entries. The ref count is handled in some places,
* but not all of them. There are some places where references get leaked
* when this scheduler entry gets cancelled. At worst, this would cause
* memory leaks on reloads if registrations get removed from configuration.
*/
struct sip_registry {
ASTOBJ_COMPONENTS_FULL(struct sip_registry,1,1);
......@@ -11223,7 +11217,7 @@ static int sip_reregister(const void *data)
 
r->expire = -1;
__sip_do_register(r);
registry_unref(r, "unreg the re-registered");
registry_unref(r, "unref the re-register scheduled event");
return 0;
}
 
......@@ -18172,7 +18166,7 @@ static int handle_response_register(struct sip_pvt *p, int resp, const char *res
break;
case 403: /* Forbidden */
ast_log(LOG_WARNING, "Forbidden - wrong password on authentication for REGISTER for '%s' to '%s'\n", p->registry->username, p->registry->hostname);
AST_SCHED_DEL(sched, r->timeout);
AST_SCHED_DEL_UNREF(sched, r->timeout, registry_unref(r, "reg ptr unref from handle_response_register 403"));
r->regstate = REG_STATE_NOAUTH;
pvt_set_needdestroy(p, "received 403 response");
break;
......@@ -18182,7 +18176,7 @@ static int handle_response_register(struct sip_pvt *p, int resp, const char *res
if (r->call)
r->call = dialog_unref(r->call, "unsetting registry->call pointer-- case 404");
r->regstate = REG_STATE_REJECTED;
AST_SCHED_DEL(sched, r->timeout);
AST_SCHED_DEL_UNREF(sched, r->timeout, registry_unref(r, "reg ptr unref from handle_response_register 404"));
break;
case 407: /* Proxy auth */
if (p->authtries == MAX_AUTHTRIES || do_register_auth(p, req, resp)) {
......@@ -18201,8 +18195,7 @@ static int handle_response_register(struct sip_pvt *p, int resp, const char *res
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", p->registry->username, p->registry->hostname, r->expiry);
AST_SCHED_DEL(sched, r->timeout);
r->timeout = -1;
AST_SCHED_DEL_UNREF(sched, r->timeout, registry_unref(r, "reg ptr unref from handle_response_register 423"));
if (r->call) {
r->call = dialog_unref(r->call, "unsetting registry->call pointer-- case 423");
pvt_set_needdestroy(p, "received 423 response");
......@@ -18223,7 +18216,7 @@ static int handle_response_register(struct sip_pvt *p, int resp, const char *res
if (r->call)
r->call = dialog_unref(r->call, "unsetting registry->call pointer-- case 479");
r->regstate = REG_STATE_REJECTED;
AST_SCHED_DEL(sched, r->timeout);
AST_SCHED_DEL_UNREF(sched, r->timeout, registry_unref(r, "reg ptr unref from handle_response_register 479"));
break;
case 200: /* 200 OK */
if (!r) {
......@@ -18240,7 +18233,7 @@ static int handle_response_register(struct sip_pvt *p, int resp, const char *res
if (r->timeout > -1) {
ast_debug(1, "Cancelling timeout %d\n", r->timeout);
}
AST_SCHED_DEL(sched, r->timeout);
AST_SCHED_DEL_UNREF(sched, r->timeout, registry_unref(r, "reg ptr unref from handle_response_register 200"));
if (r->call)
r->call = dialog_unref(r->call, "unsetting registry->call pointer-- case 200");
p->registry = registry_unref(p->registry, "unref registry entry p->registry");
......@@ -18248,14 +18241,12 @@ static int handle_response_register(struct sip_pvt *p, int resp, const char *res
sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT);
/* p->needdestroy = 1; */
/* set us up for re-registering */
/* figure out how long we got registered for */
AST_SCHED_DEL(sched, r->expire);
/* according to section 6.13 of RFC, contact headers override
expires headers, so check those first */
/* set us up for re-registering
* figure out how long we got registered for
* 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;
......@@ -18299,9 +18290,6 @@ static int handle_response_register(struct sip_pvt *p, int resp, const char *res
registry_unref(_data,"unref in REPLACE del fail"),
registry_unref(r,"unref in REPLACE add fail"),
registry_addref(r,"The Addition side of REPLACE"));
/* it is clear that we would not want to destroy the registry entry if we just
scheduled a callback and recorded it in there! */
/* since we never bumped the count, we shouldn't decrement it! registry_unref(r, "unref registry ptr r"); if this gets deleted, p->registry will be a bad pointer! */
}
return 1;
}
......@@ -24319,18 +24307,19 @@ static int reload_config(enum channelreloadreason reason)
/* First, destroy all outstanding registry calls */
/* This is needed, since otherwise active registry entries will not be destroyed */
ASTOBJ_CONTAINER_TRAVERSE(&regl, 1, do { /* regl is locked */
/* avoid a deadlock in the unlink_all call, if iterator->call's (a dialog) registry entry
is this registry entry. In other words, if the dialog we are pointing to points back to
us, then if we get a lock on this object, and try to UNREF it, we will deadlock, because
we already ... NO. This is not the problem. */
ASTOBJ_RDLOCK(iterator); /* now regl is locked, and the object is also locked */
if (iterator->call) {
ast_debug(3, "Destroying active SIP dialog for registry %s@%s\n", iterator->username, iterator->hostname);
/* This will also remove references to the registry */
dialog_unlink_all(iterator->call, TRUE, TRUE);
iterator->call = dialog_unref(iterator->call, "remove iterator->call from registry traversal");
/* iterator->call = sip_destroy(iterator->call); */
}
if (iterator->expire > -1) {
AST_SCHED_DEL_UNREF(sched, iterator->expire, registry_unref(iterator, "reg ptr unref from reload config"));
}
if (iterator->timeout > -1) {
AST_SCHED_DEL_UNREF(sched, iterator->timeout, registry_unref(iterator, "reg ptr unref from reload config"));
}
ASTOBJ_UNLOCK(iterator);
} while(0));
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment