[svn-commits] tilghman: trunk r266682 - /trunk/main/manager.c

SVN commits to the Digium repositories svn-commits at lists.digium.com
Tue Jun 1 11:41:05 CDT 2010


Author: tilghman
Date: Tue Jun  1 11:41:00 2010
New Revision: 266682

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=266682
Log:
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

Modified:
    trunk/main/manager.c

Modified: trunk/main/manager.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/manager.c?view=diff&rev=266682&r1=266681&r2=266682
==============================================================================
--- trunk/main/manager.c (original)
+++ trunk/main/manager.c Tue Jun  1 11:41:00 2010
@@ -734,11 +734,12 @@
 	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;
@@ -863,8 +864,6 @@
 
 static AST_RWLIST_HEAD_STATIC(channelvars, manager_channel_variable);
 
-#define NEW_EVENT(m)	(AST_LIST_NEXT(m->session->last_ev, eq_next))
-
 /*! \brief user descriptor, as read from the config file.
  *
  * \note It is still missing some fields -- e.g. we can have multiple permit and deny
@@ -893,8 +892,6 @@
 /*! \brief list of hooks registered */
 static AST_RWLIST_HEAD_STATIC(manager_hooks, manager_custom_hook);
 
-static struct eventqent *unref_event(struct eventqent *e);
-static void ref_event(struct eventqent *e);
 static void free_channelvars(void);
 
 /*! \brief Add a custom hook to be called when an event is fired */
@@ -931,15 +928,15 @@
 {
 	struct eventqent *ret;
 
-	AST_LIST_LOCK(&all_events);
-	ret = AST_LIST_LAST(&all_events);
+	AST_RWLIST_WRLOCK(&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) {
 		ast_atomic_fetchadd_int(&ret->usecount, 1);
 	}
-	AST_LIST_UNLOCK(&all_events);
+	AST_RWLIST_UNLOCK(&all_events);
 	return ret;
 }
 
@@ -950,14 +947,24 @@
 static void purge_events(void)
 {
 	struct eventqent *ev;
-
-	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);
+	struct timeval now = ast_tvnow();
+
+	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);
 }
 
 /*!
@@ -1107,7 +1114,7 @@
 	if (session->f != NULL) {
 		fclose(session->f);
 	}
-	unref_event(eqe);
+	ast_atomic_fetchadd_int(&eqe->usecount, -1);
 }
 
 /*! \brief Allocate manager session structure and add it to the list of sessions */
@@ -1482,13 +1489,13 @@
 	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;
 }
@@ -1513,15 +1520,17 @@
 	return CLI_SUCCESS;
 }
 
-static struct eventqent *unref_event(struct eventqent *e)
-{
-	ast_atomic_fetchadd_int(&e->usecount, -1);
-	return AST_LIST_NEXT(e, eq_next);
-}
-
-static void ref_event(struct eventqent *e)
-{
-	ast_atomic_fetchadd_int(&e->usecount, 1);
+static struct eventqent *advance_event(struct eventqent *e)
+{
+	struct eventqent *next;
+
+	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;
 }
 
 /*
@@ -2610,7 +2619,7 @@
 
 	for (x = 0; x < timeout || timeout < 0; x++) {
 		mansession_lock(s);
-		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.
@@ -2639,16 +2648,17 @@
 
 	mansession_lock(s);
 	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);
+		AST_RWLIST_RDLOCK(&all_events);
+		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;
+		}
+		AST_RWLIST_UNLOCK(&all_events);
 		astman_append(s,
 			"Event: WaitEventComplete\r\n"
 			"%s"
@@ -3622,18 +3632,19 @@
 
 	ao2_lock(s->session);
 	if (s->session->f != NULL) {
-		struct eventqent *eqe;
-
-		while ( (eqe = NEW_EVENT(s)) ) {
-			ref_event(eqe);
+		struct eventqent *eqe = s->session->last_ev;
+		AST_RWLIST_RDLOCK(&all_events);
+		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)
+				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_RWLIST_UNLOCK(&all_events);
 	}
 	ao2_unlock(s->session);
 	return ret;
@@ -4251,12 +4262,13 @@
 	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;
 }




More information about the svn-commits mailing list