Merged revisions 266682 via svnmerge from

https://origsvn.digium.com/svn/asterisk/trunk

........
  r266682 | tilghman | 2010-06-01 11:41:00 -0500 (Tue, 01 Jun 2010) | 16 lines

  Eliminate stale manager events after a set interval, even if AMI clients don't query for them.

  Actions (or failures to act) by external clients should not cause memory leaks
  in Asterisk, especially when those continued leaks could cause Asterisk to
  misbehave later.

  (closes issue #17234)
   Reported by: mav3rick
   Patches:
         20100510__issue17234.diff.txt uploaded by tilghman (license 14)
         20100517__issue17234__trunk.diff.txt uploaded by tilghman (license 14)
   Tested by: mav3rick, davidw

  (closes issue #17365)
   Reported by: davidw
........

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

diff --git a/main/manager.c b/main/manager.c
index 66253c543034674e809791a269572c5564a87cc0..ab2877556d83a1860b2c8326b1ae25208c9be9e7 100644 (file)
--- a/main/manager.c
+++ b/main/manager.c
@@ -113,11 +113,12 @@ struct eventqent {
        int usecount;           /*!< # of clients who still need the event */
        int category;
        unsigned int seq;       /*!< sequence number */
-       AST_LIST_ENTRY(eventqent) eq_next;
+       struct timeval tv;  /*!< When event was allocated */
+       AST_RWLIST_ENTRY(eventqent) eq_next;
        char eventdata[1];      /*!< really variable size, allocated by append_event() */
 };
 
-static AST_LIST_HEAD_STATIC(all_events, eventqent);
+static AST_RWLIST_HEAD_STATIC(all_events, eventqent);
 
 static int displayconnects = 1;
 static int allowmultiplelogin = 1;
@@ -222,8 +223,6 @@ struct mansession {
        int fd;
 };
 
-#define NEW_EVENT(m)   (AST_LIST_NEXT(m->session->last_ev, eq_next))
-
 static AST_LIST_HEAD_STATIC(sessions, mansession_session);
 
 /*! \brief user descriptor, as read from the config file.
@@ -332,14 +331,15 @@ static struct eventqent *grab_last(void)
 {
        struct eventqent *ret;
 
-       AST_LIST_LOCK(&all_events);
-       ret = AST_LIST_LAST(&all_events);
+       AST_RWLIST_RDLOCK(&all_events);
+       ret = AST_RWLIST_LAST(&all_events);
        /* the list is never empty now, but may become so when
         * we optimize it in the future, so be prepared.
         */
-       if (ret)
+       if (ret) {
                ast_atomic_fetchadd_int(&ret->usecount, 1);
-       AST_LIST_UNLOCK(&all_events);
+       }
+       AST_RWLIST_UNLOCK(&all_events);
        return ret;
 }
 
@@ -350,14 +350,24 @@ static struct eventqent *grab_last(void)
 static void purge_events(void)
 {
        struct eventqent *ev;
+       struct timeval now = ast_tvnow();
 
-       AST_LIST_LOCK(&all_events);
-       while ( (ev = AST_LIST_FIRST(&all_events)) &&
-           ev->usecount == 0 && AST_LIST_NEXT(ev, eq_next)) {
-               AST_LIST_REMOVE_HEAD(&all_events, eq_next);
+       AST_RWLIST_WRLOCK(&all_events);
+       while ( (ev = AST_RWLIST_FIRST(&all_events)) &&
+           ev->usecount == 0 && AST_RWLIST_NEXT(ev, eq_next)) {
+               AST_RWLIST_REMOVE_HEAD(&all_events, eq_next);
                ast_free(ev);
        }
-       AST_LIST_UNLOCK(&all_events);
+
+       AST_RWLIST_TRAVERSE_SAFE_BEGIN(&all_events, ev, eq_next) {
+               /* 2.5 times whatever the HTTP timeout is (maximum 2.5 hours) is the maximum time that we will definitely cache an event */
+               if (ev->usecount == 0 && ast_tvdiff_sec(now, ev->tv) > (httptimeout > 3600 ? 3600 : httptimeout) * 2.5) {
+                       AST_RWLIST_REMOVE_CURRENT(eq_next);
+                       ast_free(ev);
+               }
+       }
+       AST_RWLIST_TRAVERSE_SAFE_END;
+       AST_RWLIST_UNLOCK(&all_events);
 }
 
 /*!
@@ -770,13 +780,13 @@ static char *handle_showmaneventq(struct ast_cli_entry *e, int cmd, struct ast_c
        case CLI_GENERATE:
                return NULL;
        }
-       AST_LIST_LOCK(&all_events);
-       AST_LIST_TRAVERSE(&all_events, s, eq_next) {
+       AST_RWLIST_RDLOCK(&all_events);
+       AST_RWLIST_TRAVERSE(&all_events, s, eq_next) {
                ast_cli(a->fd, "Usecount: %d\n", s->usecount);
                ast_cli(a->fd, "Category: %d\n", s->category);
                ast_cli(a->fd, "Event:\n%s", s->eventdata);
        }
-       AST_LIST_UNLOCK(&all_events);
+       AST_RWLIST_UNLOCK(&all_events);
 
        return CLI_SUCCESS;
 }
@@ -812,21 +822,17 @@ static struct ast_cli_entry cli_manager[] = {
        AST_CLI_DEFINE(handle_manager_reload, "Reload manager configurations"),
 };
 
-/*
- * Decrement the usecount for the event; if it goes to zero,
- * (why check for e->next ?) wakeup the
- * main thread, which is in charge of freeing the record.
- * Returns the next record.
- */
-static struct eventqent *unref_event(struct eventqent *e)
+static struct eventqent *advance_event(struct eventqent *e)
 {
-       ast_atomic_fetchadd_int(&e->usecount, -1);
-       return AST_LIST_NEXT(e, eq_next);
-}
+       struct eventqent *next;
 
-static void ref_event(struct eventqent *e)
-{
-       ast_atomic_fetchadd_int(&e->usecount, 1);
+       AST_RWLIST_RDLOCK(&all_events);
+       if ((next = AST_RWLIST_NEXT(e, eq_next))) {
+               ast_atomic_fetchadd_int(&next->usecount, 1);
+               ast_atomic_fetchadd_int(&e->usecount, -1);
+       }
+       AST_RWLIST_UNLOCK(&all_events);
+       return next;
 }
 
 /*
@@ -847,7 +853,7 @@ static void free_session(struct mansession_session *session)
                fclose(session->f);
        ast_mutex_destroy(&session->__lock);
        ast_free(session);
-       unref_event(eqe);
+       ast_atomic_fetchadd_int(&eqe->usecount, -1);
 }
 
 static void destroy_session(struct mansession_session *session)
@@ -1638,8 +1644,9 @@ static int action_waitevent(struct mansession *s, const struct message *m)
 
        for (x = 0; x < timeout || timeout < 0; x++) {
                ast_mutex_lock(&s->session->__lock);
-               if (NEW_EVENT(s))
+               if (AST_RWLIST_NEXT(s->session->last_ev, eq_next)) {
                        needexit = 1;
+               }
                /* We can have multiple HTTP session point to the same mansession entry.
                 * The way we deal with it is not very nice: newcomers kick out the previous
                 * HTTP session. XXX this needs to be improved.
@@ -1661,15 +1668,14 @@ static int action_waitevent(struct mansession *s, const struct message *m)
        ast_debug(1, "Finished waiting for an event!\n");
        ast_mutex_lock(&s->session->__lock);
        if (s->session->waiting_thread == pthread_self()) {
-               struct eventqent *eqe;
+               struct eventqent *eqe = s->session->last_ev;
                astman_send_response(s, m, "Success", "Waiting for Event completed.");
-               while ( (eqe = NEW_EVENT(s)) ) {
-                       ref_event(eqe);
+               while ((eqe = advance_event(eqe))) {
                        if (((s->session->readperm & eqe->category) == eqe->category) &&
                            ((s->session->send_events & eqe->category) == eqe->category)) {
                                astman_append(s, "%s", eqe->eventdata);
                        }
-                       s->session->last_ev = unref_event(s->session->last_ev);
+                       s->session->last_ev = eqe;
                }
                astman_append(s,
                        "Event: WaitEventComplete\r\n"
@@ -2682,17 +2688,16 @@ static int process_events(struct mansession *s)
 
        ast_mutex_lock(&s->session->__lock);
        if (s->session->f != NULL) {
-               struct eventqent *eqe;
+               struct eventqent *eqe = s->session->last_ev;
 
-               while ( (eqe = NEW_EVENT(s)) ) {
-                       ref_event(eqe);
+               while ((eqe = advance_event(eqe))) {
                        if (!ret && s->session->authenticated &&
                            (s->session->readperm & eqe->category) == eqe->category &&
                            (s->session->send_events & eqe->category) == eqe->category) {
                                if (send_string(s, eqe->eventdata) < 0)
                                        ret = -1;       /* don't send more */
                        }
-                       s->session->last_ev = unref_event(s->session->last_ev);
+                       s->session->last_ev = eqe;
                }
        }
        ast_mutex_unlock(&s->session->__lock);
@@ -3307,12 +3312,13 @@ static int append_event(const char *str, int category)
        tmp->usecount = 0;
        tmp->category = category;
        tmp->seq = ast_atomic_fetchadd_int(&seq, 1);
-       AST_LIST_NEXT(tmp, eq_next) = NULL;
+       tmp->tv = ast_tvnow();
+       AST_RWLIST_NEXT(tmp, eq_next) = NULL;
        strcpy(tmp->eventdata, str);
 
-       AST_LIST_LOCK(&all_events);
-       AST_LIST_INSERT_TAIL(&all_events, tmp, eq_next);
-       AST_LIST_UNLOCK(&all_events);
+       AST_RWLIST_WRLOCK(&all_events);
+       AST_RWLIST_INSERT_TAIL(&all_events, tmp, eq_next);
+       AST_RWLIST_UNLOCK(&all_events);
 
        return 0;
 }