Merged revisions 318549 via svnmerge from
authorTerry Wilson <twilson@digium.com>
Wed, 11 May 2011 18:50:51 +0000 (18:50 +0000)
committerTerry Wilson <twilson@digium.com>
Wed, 11 May 2011 18:50:51 +0000 (18:50 +0000)
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/
  ........
................

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@318551 65c4cc65-6c06-0410-ace0-fbb531ad65f3

channels/chan_sip.c

index af83c8a..394a5f1 100644 (file)
@@ -228,7 +228,7 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
    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 @@ cleanup:
        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)
+#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)
 {
-       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");
+       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;
 }
 
-/* \brief Unlink single peer from all ao2 containers */
-static void unlink_peer_from_tables(struct sip_peer *peer)
+static struct sip_peer *_unref_peer(struct sip_peer *peer, char *tag, char *file, int line, const char *func)
 {
-       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");
-       }
+       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
@@ -2835,6 +2824,66 @@ static struct sip_peer *ref_peer(struct sip_peer *peer, char *tag)
        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
  *
@@ -2947,6 +2996,10 @@ void *dialog_unlink_all(struct sip_pvt *dialog, int lockowner, int lockdialoglis
                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");
        return NULL;
 }
@@ -3879,22 +3932,15 @@ void sip_scheddestroy(struct sip_pvt *p, int ms)
  */
 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");
-               }
+               append_history(p, "CancelDestroy", "");
+               AST_SCHED_DEL_UNREF(sched, p->autokillid, dialog_unref(p, "remove ref for autokillid"));
        }
-       return res;
+       return 0;
 }
 
 /*! \brief Acknowledges receipt of a packet and stops retransmission
@@ -5496,7 +5542,10 @@ void __sip_destroy(struct sip_pvt *p, int lockowner, int lockdialoglist)
 {
        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;
        }
@@ -5572,17 +5621,6 @@ void __sip_destroy(struct sip_pvt *p, int lockowner, int lockdialoglist)
        }
        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) {
                struct sip_history *hist;
@@ -6537,7 +6575,7 @@ static int interpret_t38_parameters(struct sip_pvt *p, const struct ast_control_
                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, &parameters, sizeof(parameters));
@@ -7798,7 +7836,7 @@ static struct ast_channel *sip_pvt_lock_full(struct sip_pvt *pvt)
 /*! \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 @@ static char *sip_show_users(struct ast_cli_entry *e, int cmd, struct ast_cli_arg
        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 @@ static char *_sip_show_peers(int fd, int *total, struct mansession *s, const str
                ao2_lock(peer);
                if (havepattern && regexec(&regexbuf, 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 @@ static char *_sip_show_peers(int fd, int *total, struct mansession *s, const str
                        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 @@ static char *complete_sip_user(const char *word, int state)
        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 @@ static int handle_request_invite(struct sip_pvt *p, struct sip_request *req, int
                        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 @@ static int handle_request_cancel(struct sip_pvt *p, struct sip_request *req)
                        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;
                        }
@@ -24073,6 +24119,10 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req,
                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) {
                if (p->stateid > -1) {
@@ -25077,9 +25127,9 @@ static void restart_session_timer(struct sip_pvt *p)
        }
 
        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 @@ static void stop_session_timer(struct sip_pvt *p)
 
        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 @@ static void start_session_timer(struct sip_pvt *p)
                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_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);
        }
-       ast_debug(2, "Session timer started: %d - %s\n", p->stimer->st_schedid, p->callid);
 }
 
 
@@ -25188,6 +25247,7 @@ return_unref:
        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 @@ static int sip_poke_peer(struct sip_peer *peer, int force)
 #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 @@ static int unload_module(void)
        }
        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 @@ static int unload_module(void)
        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);