[asterisk-commits] twilson: trunk r318551 - in /trunk: ./ channels/chan_sip.c
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Wed May 11 13:51:10 CDT 2011
Author: twilson
Date: Wed May 11 13:50:51 2011
New Revision: 318551
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=318551
Log:
Merged revisions 318549 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.8
................
r318549 | twilson | 2011-05-11 13:39:48 -0500 (Wed, 11 May 2011) | 27 lines
Merged revisions 318548 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.6.2
........
r318548 | twilson | 2011-05-11 12:15:39 -0500 (Wed, 11 May 2011) | 19 lines
Clean up several chan_sip reference leaks
Several situations in the code could lead to peers or sip_pvt references
being leaked. This would cause RTP ports to never be destroyed (leading
to exhaustion of all available RTP ports) and memory leaks.
The original patch for this issue from rgagnon was the result of an
obscene amount of testing and hard work, for which I am very grateful. I
did some cleanup and added a few additional refcount fixes that I found.
(closes issue #17255)
Reported by: kvveltho
Patches:
tag-1.6.2.17-r309252-sip-dos-mem-leak-fix.diff uploaded by rgagnon (license 1202)
Tested by: rgagnon, twilson, wdoekes, loloski
Review: https://reviewboard.asterisk.org/r/1101/
Review: https://reviewboard.asterisk.org/r/1207/
Review: https://reviewboard.asterisk.org/r/1210/
........
................
Modified:
trunk/ (props changed)
trunk/channels/chan_sip.c
Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-1.8-merged' - no diff available.
Modified: trunk/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/trunk/channels/chan_sip.c?view=diff&rev=318551&r1=318550&r2=318551
==============================================================================
--- trunk/channels/chan_sip.c (original)
+++ trunk/channels/chan_sip.c Wed May 11 13:50:51 2011
@@ -228,7 +228,7 @@
In normal operation, the macros defined will throw away the tags, so they do not
affect the speed of the program at all. They can be considered to be documentation.
*/
-/* #define REF_DEBUG 1 */
+#define REF_DEBUG 1
#include "asterisk/lock.h"
#include "asterisk/config.h"
#include "asterisk/module.h"
@@ -2789,36 +2789,25 @@
return NULL;
}
-/* this func is used with ao2_callback to unlink/delete all marked
- peers */
-static int peer_is_marked(void *peerobj, void *arg, int flags)
-{
- struct sip_peer *peer = peerobj;
- if (peer->the_mark && peer->pokeexpire != -1) {
- AST_SCHED_DEL(sched, peer->pokeexpire);
- }
- return peer->the_mark ? CMP_MATCH : 0;
-}
-
-
-/* \brief Unlink all marked peers from ao2 containers */
-static void unlink_marked_peers_from_tables(void)
-{
- ao2_t_callback(peers, OBJ_NODATA | OBJ_UNLINK | OBJ_MULTIPLE, peer_is_marked, NULL,
- "initiating callback to remove marked peers");
- ao2_t_callback(peers_by_ip, OBJ_NODATA | OBJ_UNLINK | OBJ_MULTIPLE, peer_is_marked, NULL,
- "initiating callback to remove marked peers");
-}
-
-/* \brief Unlink single peer from all ao2 containers */
-static void unlink_peer_from_tables(struct sip_peer *peer)
-{
- ao2_t_unlink(peers, peer, "ao2_unlink of peer from peers table");
- if (!ast_sockaddr_isnull(&peer->addr)) {
- ao2_t_unlink(peers_by_ip, peer, "ao2_unlink of peer from peers_by_ip table");
- }
-}
-
+#ifdef REF_DEBUG
+#define ref_peer(arg1,arg2) _ref_peer((arg1),(arg2), __FILE__, __LINE__, __PRETTY_FUNCTION__)
+#define unref_peer(arg1,arg2) _unref_peer((arg1),(arg2), __FILE__, __LINE__, __PRETTY_FUNCTION__)
+static struct sip_peer *_ref_peer(struct sip_peer *peer, char *tag, char *file, int line, const char *func)
+{
+ if (peer)
+ __ao2_ref_debug(peer, 1, tag, file, line, func);
+ else
+ ast_log(LOG_ERROR, "Attempt to Ref a null peer pointer\n");
+ return peer;
+}
+
+static struct sip_peer *_unref_peer(struct sip_peer *peer, char *tag, char *file, int line, const char *func)
+{
+ if (peer)
+ __ao2_ref_debug(peer, -1, tag, file, line, func);
+ return NULL;
+}
+#else
/*!
* helper functions to unreference various types of objects.
* By handling them this way, we don't have to declare the
@@ -2834,6 +2823,66 @@
{
ao2_t_ref(peer, 1, tag);
return peer;
+}
+#endif /* REF_DEBUG */
+
+static void peer_sched_cleanup(struct sip_peer *peer)
+{
+ if (peer->pokeexpire != -1) {
+ AST_SCHED_DEL_UNREF(sched, peer->pokeexpire,
+ unref_peer(peer, "removing poke peer ref"));
+ }
+ if (peer->expire != -1) {
+ AST_SCHED_DEL_UNREF(sched, peer->expire,
+ unref_peer(peer, "remove register expire ref"));
+ }
+}
+
+typedef enum {
+ SIP_PEERS_MARKED,
+ SIP_PEERS_ALL,
+} peer_unlink_flag_t;
+
+/* this func is used with ao2_callback to unlink/delete all marked or linked
+ peers, depending on arg */
+static int match_and_cleanup_peer_sched(void *peerobj, void *arg, int flags)
+{
+ struct sip_peer *peer = peerobj;
+ peer_unlink_flag_t which = *(peer_unlink_flag_t *)arg;
+
+ if (which == SIP_PEERS_ALL || peer->the_mark) {
+ peer_sched_cleanup(peer);
+ return CMP_MATCH;
+ }
+ return 0;
+}
+
+static void unlink_peers_from_tables(peer_unlink_flag_t flag)
+{
+ ao2_t_callback(peers, OBJ_NODATA | OBJ_UNLINK | OBJ_MULTIPLE,
+ match_and_cleanup_peer_sched, &flag, "initiating callback to remove marked peers");
+ ao2_t_callback(peers_by_ip, OBJ_NODATA | OBJ_UNLINK | OBJ_MULTIPLE,
+ match_and_cleanup_peer_sched, &flag, "initiating callback to remove marked peers");
+}
+
+/* \brief Unlink all marked peers from ao2 containers */
+static void unlink_marked_peers_from_tables(void)
+{
+ unlink_peers_from_tables(SIP_PEERS_MARKED);
+}
+
+static void unlink_all_peers_from_tables(void)
+{
+ unlink_peers_from_tables(SIP_PEERS_ALL);
+}
+
+/* \brief Unlink single peer from all ao2 containers */
+static void unlink_peer_from_tables(struct sip_peer *peer)
+{
+ ao2_t_unlink(peers, peer, "ao2_unlink of peer from peers table");
+ if (!ast_sockaddr_isnull(&peer->addr)) {
+ ao2_t_unlink(peers_by_ip, peer, "ao2_unlink of peer from peers_by_ip table");
+ }
}
/*! \brief maintain proper refcounts for a sip_pvt's outboundproxy
@@ -2945,6 +2994,10 @@
if (dialog->t38id > -1) {
AST_SCHED_DEL_UNREF(sched, dialog->t38id, dialog_unref(dialog, "when you delete the t38id sched, you should dec the refcount for the stored dialog ptr"));
+ }
+
+ if (dialog->stimer) {
+ stop_session_timer(dialog);
}
dialog_unref(dialog, "Let's unbump the count in the unlink so the poor pvt can disappear if it is time");
@@ -3879,22 +3932,15 @@
*/
int sip_cancel_destroy(struct sip_pvt *p)
{
- int res = 0;
-
if (p->final_destruction_scheduled) {
- return res;
+ return 0;
}
if (p->autokillid > -1) {
- int res3;
-
- if (!(res3 = ast_sched_del(sched, p->autokillid))) {
- append_history(p, "CancelDestroy", "");
- p->autokillid = -1;
- dialog_unref(p, "dialog unrefd because autokillid is de-sched'd");
- }
- }
- return res;
+ append_history(p, "CancelDestroy", "");
+ AST_SCHED_DEL_UNREF(sched, p->autokillid, dialog_unref(p, "remove ref for autokillid"));
+ }
+ return 0;
}
/*! \brief Acknowledges receipt of a packet and stops retransmission
@@ -5496,7 +5542,10 @@
{
struct sip_request *req;
- if (p->stimer) {
+ /* Destroy Session-Timers if allocated */
+ if (p->stimer) {
+ p->stimer->quit_flag = 1;
+ stop_session_timer(p);
ast_free(p->stimer);
p->stimer = NULL;
}
@@ -5571,17 +5620,6 @@
p->route = NULL;
}
deinit_req(&p->initreq);
-
- /* Destroy Session-Timers if allocated */
- if (p->stimer) {
- p->stimer->quit_flag = 1;
- if (p->stimer->st_active == TRUE && p->stimer->st_schedid > -1) {
- AST_SCHED_DEL_UNREF(sched, p->stimer->st_schedid,
- dialog_unref(p, "removing session timer ref"));
- }
- ast_free(p->stimer);
- p->stimer = NULL;
- }
/* Clear history */
if (p->history) {
@@ -6537,7 +6575,7 @@
struct ast_control_t38_parameters parameters = p->t38.their_parms;
if (p->t38.state == T38_PEER_REINVITE) {
- AST_SCHED_DEL(sched, p->t38id);
+ AST_SCHED_DEL_UNREF(sched, p->t38id, dialog_unref(p, "when you delete the t38id sched, you should dec the refcount for the stored dialog ptr"));
parameters.max_ifp = ast_udptl_get_far_max_ifp(p->udptl);
parameters.request_response = AST_T38_REQUEST_NEGOTIATE;
ast_queue_control_data(p->owner, AST_CONTROL_T38_PARAMETERS, ¶meters, sizeof(parameters));
@@ -7798,7 +7836,7 @@
/*! \brief find or create a dialog structure for an incoming SIP message.
* Connect incoming SIP message to current dialog or create new dialog structure
* Returns a reference to the sip_pvt object, remember to give it back once done.
- * Called by handle_incoming(), sipsock_read
+ * Called by handle_request_do
*/
static struct sip_pvt *find_call(struct sip_request *req, struct ast_sockaddr *addr, const int intended_method)
{
@@ -16070,7 +16108,7 @@
ast_cli(a->fd, FORMAT, "Username", "Secret", "Accountcode", "Def.Context", "ACL", "ForcerPort");
user_iter = ao2_iterator_init(peers, 0);
- while ((user = ao2_iterator_next(&user_iter))) {
+ while ((user = ao2_t_iterator_next(&user_iter, "iterate thru peers table"))) {
ao2_lock(user);
if (!(user->type & SIP_TYPE_USER)) {
ao2_unlock(user);
@@ -16290,7 +16328,7 @@
ao2_lock(peer);
if (havepattern && regexec(®exbuf, peer->name, 0, NULL, 0)) {
ao2_unlock(peer);
- unref_peer(peer, "toss iterator peer ptr before continue");
+ peer = peerarray[k] = unref_peer(peer, "toss iterator peer ptr before continue");
continue;
}
@@ -16362,7 +16400,7 @@
peer->description);
}
ao2_unlock(peer);
- unref_peer(peer, "toss iterator peer ptr");
+ peer = peerarray[k] = unref_peer(peer, "toss iterator peer ptr");
}
if (!s)
@@ -17177,7 +17215,7 @@
struct sip_peer *user;
user_iter = ao2_iterator_init(peers, 0);
- while ((user = ao2_iterator_next(&user_iter))) {
+ while ((user = ao2_t_iterator_next(&user_iter, "iterate thru peers table"))) {
ao2_lock(user);
if (!(user->type & SIP_TYPE_USER)) {
ao2_unlock(user);
@@ -22362,6 +22400,10 @@
transmit_response(p, "100 Trying", req);
if (p->t38.state == T38_PEER_REINVITE) {
+ if (p->t38id > -1) {
+ /* reset t38 abort timer */
+ AST_SCHED_DEL_UNREF(sched, p->t38id, dialog_unref(p, "remove ref for t38id"));
+ }
p->t38id = ast_sched_add(sched, 5000, sip_t38_abort, dialog_ref(p, "passing dialog ptr into sched structure based on t38id for sip_t38_abort."));
} else if (p->t38.state == T38_ENABLED) {
ast_set_flag(&p->flags[1], SIP_PAGE2_DIALOG_ESTABLISHED);
@@ -23060,6 +23102,10 @@
if (pkt->seqno == p->lastinvite && pkt->response_code == 487) {
AST_SCHED_DEL(sched, pkt->retransid);
UNLINK(pkt, p->packets, prev_pkt);
+ dialog_unref(pkt->owner, "unref packet->owner from dialog");
+ if (pkt->data) {
+ ast_free(pkt->data);
+ }
ast_free(pkt);
break;
}
@@ -24072,6 +24118,10 @@
unref_peer(authpeer, "unref_peer, from handle_request_subscribe (authpeer 5)");
return 0;
}
+
+ /* At this point, if we have an authpeer (which we have to have to get here) we should unref
+ * it since if we have actually used it, we have reffed it when p->relatedpeer was set. */
+ authpeer = unref_peer(authpeer, "unref pointer into (*authpeer)");
/* Add subscription for extension state from the PBX core */
if (p->subscribed != MWI_NOTIFICATION && p->subscribed != CALL_COMPLETION && !resubscribe) {
@@ -25077,9 +25127,9 @@
}
if (p->stimer->st_active == TRUE) {
+ ast_debug(2, "Session timer stopped: %d - %s\n", p->stimer->st_schedid, p->callid);
AST_SCHED_DEL_UNREF(sched, p->stimer->st_schedid,
dialog_unref(p, "Removing session timer ref"));
- ast_debug(2, "Session timer stopped: %d - %s\n", p->stimer->st_schedid, p->callid);
start_session_timer(p);
}
}
@@ -25095,9 +25145,9 @@
if (p->stimer->st_active == TRUE) {
p->stimer->st_active = FALSE;
+ ast_debug(2, "Session timer stopped: %d - %s\n", p->stimer->st_schedid, p->callid);
AST_SCHED_DEL_UNREF(sched, p->stimer->st_schedid,
dialog_unref(p, "removing session timer ref"));
- ast_debug(2, "Session timer stopped: %d - %s\n", p->stimer->st_schedid, p->callid);
}
}
@@ -25110,13 +25160,22 @@
return;
}
- p->stimer->st_schedid = ast_sched_add(sched, p->stimer->st_interval * 1000 / 2, proc_session_timer,
+ if (p->stimer->st_schedid > -1) {
+ /* in the event a timer is already going, stop it */
+ ast_debug(2, "Session timer stopped: %d - %s\n", p->stimer->st_schedid, p->callid);
+ AST_SCHED_DEL_UNREF(sched, p->stimer->st_schedid,
+ dialog_unref(p, "unref stimer->st_schedid from dialog"));
+ }
+
+ p->stimer->st_schedid = ast_sched_add(sched, p->stimer->st_interval * 1000 / 2, proc_session_timer,
dialog_ref(p, "adding session timer ref"));
if (p->stimer->st_schedid < 0) {
dialog_unref(p, "removing session timer ref");
- ast_log(LOG_ERROR, "ast_sched_add failed.\n");
- }
- ast_debug(2, "Session timer started: %d - %s\n", p->stimer->st_schedid, p->callid);
+ ast_log(LOG_ERROR, "ast_sched_add failed - %s\n", p->callid);
+ } else {
+ p->stimer->st_active = TRUE;
+ ast_debug(2, "Session timer started: %d - %s\n", p->stimer->st_schedid, p->callid);
+ }
}
@@ -25188,6 +25247,7 @@
if (!res) {
/* An error occurred. Stop session timer processing */
if (p->stimer) {
+ ast_debug(2, "Session timer stopped: %d - %s\n", p->stimer->st_schedid, p->callid);
p->stimer->st_schedid = -1;
stop_session_timer(p);
}
@@ -25490,7 +25550,8 @@
#endif
peer->ps = ast_tvnow();
if (xmitres == XMIT_ERROR) {
- sip_poke_noanswer(peer); /* Immediately unreachable, network problems */
+ /* Immediately unreachable, network problems */
+ sip_poke_noanswer(ref_peer(peer, "add ref for peerexpire (fake, for sip_poke_noanswer to remove)"));
} else if (!force) {
AST_SCHED_REPLACE_UNREF(peer->pokeexpire, sched, peer->maxms * 2, sip_poke_noanswer, peer,
unref_peer(_data, "removing poke peer ref"),
@@ -29579,6 +29640,8 @@
}
ao2_iterator_destroy(&i);
+ unlink_all_peers_from_tables();
+
ast_mutex_lock(&monlock);
if (monitor_thread && (monitor_thread != AST_PTHREADT_STOP) && (monitor_thread != AST_PTHREADT_NULL)) {
pthread_cancel(monitor_thread);
@@ -29630,6 +29693,7 @@
ao2_t_ref(dialogs_needdestroy, -1, "unref the dialogs table");
ao2_t_ref(dialogs_rtpcheck, -1, "unref the dialogs table");
ao2_t_ref(threadt, -1, "unref the thread table");
+ ao2_t_ref(sip_monitor_instances, -1, "unref the sip_monitor_instances table");
clear_sip_domains();
ast_free_ha(sip_cfg.contact_ha);
More information about the asterisk-commits
mailing list