[svn-commits] dvossel: branch 1.6.1 r201365 - in /branches/1.6.1: ./ channels/chan_sip.c
SVN commits to the Digium repositories
svn-commits at lists.digium.com
Wed Jun 17 10:32:49 CDT 2009
Author: dvossel
Date: Wed Jun 17 10:32:43 2009
New Revision: 201365
URL: http://svn.asterisk.org/svn-view/asterisk?view=rev&rev=201365
Log:
Merged revisions 201344 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk
........
r201344 | dvossel | 2009-06-17 10:20:26 -0500 (Wed, 17 Jun 2009) | 16 lines
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/
........
Modified:
branches/1.6.1/ (props changed)
branches/1.6.1/channels/chan_sip.c
Propchange: branches/1.6.1/
------------------------------------------------------------------------------
Binary property 'trunk-merged' - no diff available.
Modified: branches/1.6.1/channels/chan_sip.c
URL: http://svn.asterisk.org/svn-view/asterisk/branches/1.6.1/channels/chan_sip.c?view=diff&rev=201365&r1=201364&r2=201365
==============================================================================
--- branches/1.6.1/channels/chan_sip.c (original)
+++ branches/1.6.1/channels/chan_sip.c Wed Jun 17 10:32:43 2009
@@ -1601,12 +1601,6 @@
* 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);
@@ -10071,7 +10065,7 @@
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;
}
@@ -16531,7 +16525,7 @@
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;
p->needdestroy = 1;
break;
@@ -16541,7 +16535,7 @@
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)) {
@@ -16560,8 +16554,7 @@
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");
p->needdestroy = 1;
@@ -16582,7 +16575,7 @@
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) {
@@ -16599,7 +16592,7 @@
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");
@@ -16607,14 +16600,12 @@
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;
@@ -16658,9 +16649,6 @@
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;
}
@@ -22426,18 +22414,19 @@
/* First, destroy all outstanding registry calls */
/* This is needed, since otherwise active registry entries will not be destroyed */
ASTOBJ_CONTAINER_TRAVERSE(®l, 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);
More information about the svn-commits
mailing list