[asterisk-commits] rmudgett: branch certified-11.2 r406347 - in /certified/branches/11.2: ./ main/

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Fri Jan 24 12:19:00 CST 2014


Author: rmudgett
Date: Fri Jan 24 12:18:58 2014
New Revision: 406347

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=406347
Log:
manager: Protect data structures during shutdown.

Occasionally, the manager module would get an "INTERNAL_OBJ: bad magic
number" error on a "core restart gracefully" command if an AMI connection
is established.

* Added ao2_global_obj protection to the sessions global container.

* Fixed the order of unreferencing a session object in session_destroy().

* Removed unnecessary container traversals of the white/black filters
during session_destructor().

(closes issue AST-1242)
Reported by: Guenther Kelleter

Review: https://reviewboard.asterisk.org/r/3144/
........

Merged revisions 406341 from http://svn.asterisk.org/svn/asterisk/branches/11

Modified:
    certified/branches/11.2/   (props changed)
    certified/branches/11.2/main/manager.c

Propchange: certified/branches/11.2/
------------------------------------------------------------------------------
Binary property 'branch-11-merged' - no diff available.

Modified: certified/branches/11.2/main/manager.c
URL: http://svnview.digium.com/svn/asterisk/certified/branches/11.2/main/manager.c?view=diff&rev=406347&r1=406346&r2=406347
==============================================================================
--- certified/branches/11.2/main/manager.c (original)
+++ certified/branches/11.2/main/manager.c Fri Jan 24 12:18:58 2014
@@ -1150,7 +1150,8 @@
 	ast_mutex_t lock;
 };
 
-static struct ao2_container *sessions = NULL;
+/*! Active manager connection sessions container. */
+static AO2_GLOBAL_OBJ_STATIC(mgr_sessions);
 
 struct manager_channel_variable {
 	AST_LIST_ENTRY(manager_channel_variable) entry;
@@ -1192,7 +1193,7 @@
 static AST_RWLIST_HEAD_STATIC(manager_hooks, manager_custom_hook);
 
 /*! \brief A container of event documentation nodes */
-AO2_GLOBAL_OBJ_STATIC(event_docs);
+static AO2_GLOBAL_OBJ_STATIC(event_docs);
 
 static void free_channelvars(void);
 
@@ -1445,7 +1446,7 @@
 	if (manager_debug) {
 		ast_debug(1, "Mansession: %p refcount now %d\n", s, refcount - 1);
 	}
-	return s;
+	return NULL;
 }
 
 static void event_filter_destructor(void *obj)
@@ -1477,12 +1478,10 @@
 	}
 
 	if (session->whitefilters) {
-		ao2_t_callback(session->whitefilters, OBJ_UNLINK | OBJ_NODATA | OBJ_MULTIPLE, NULL, NULL, "unlink all white filters");
 		ao2_t_ref(session->whitefilters, -1 , "decrement ref for white container, should be last one");
 	}
 
 	if (session->blackfilters) {
-		ao2_t_callback(session->blackfilters, OBJ_UNLINK | OBJ_NODATA | OBJ_MULTIPLE, NULL, NULL, "unlink all black filters");
 		ao2_t_ref(session->blackfilters, -1 , "decrement ref for black container, should be last one");
 	}
 }
@@ -1490,19 +1489,18 @@
 /*! \brief Allocate manager session structure and add it to the list of sessions */
 static struct mansession_session *build_mansession(const struct ast_sockaddr *addr)
 {
+	struct ao2_container *sessions;
 	struct mansession_session *newsession;
 
-	if (!(newsession = ao2_alloc(sizeof(*newsession), session_destructor))) {
+	newsession = ao2_alloc(sizeof(*newsession), session_destructor);
+	if (!newsession) {
 		return NULL;
 	}
 
-	if (!(newsession->whitefilters = ao2_container_alloc(1, NULL, NULL))) {
+	newsession->whitefilters = ao2_container_alloc(1, NULL, NULL);
+	newsession->blackfilters = ao2_container_alloc(1, NULL, NULL);
+	if (!newsession->whitefilters || !newsession->blackfilters) {
 		ao2_ref(newsession, -1);
-		return NULL;
-	}
-
-	if (!(newsession->blackfilters = ao2_container_alloc(1, NULL, NULL))) {
-		ao2_ref(newsession, -1); /* session_destructor will cleanup the other filter */
 		return NULL;
 	}
 
@@ -1512,7 +1510,11 @@
 	newsession->send_events = -1;
 	ast_sockaddr_copy(&newsession->addr, addr);
 
-	ao2_link(sessions, newsession);
+	sessions = ao2_global_obj_ref(mgr_sessions);
+	if (sessions) {
+		ao2_link(sessions, newsession);
+		ao2_ref(sessions, -1);
+	}
 
 	return newsession;
 }
@@ -1526,19 +1528,31 @@
 
 static void session_destroy(struct mansession_session *s)
 {
+	struct ao2_container *sessions;
+
+	sessions = ao2_global_obj_ref(mgr_sessions);
+	if (sessions) {
+		ao2_unlink(sessions, s);
+		ao2_ref(sessions, -1);
+	}
 	unref_mansession(s);
-	ao2_unlink(sessions, s);
 }
 
 
 static int check_manager_session_inuse(const char *name)
 {
-	struct mansession_session *session = ao2_find(sessions, (char *) name, 0);
+	struct ao2_container *sessions;
+	struct mansession_session *session;
 	int inuse = 0;
 
-	if (session) {
-		inuse = 1;
-		unref_mansession(session);
+	sessions = ao2_global_obj_ref(mgr_sessions);
+	if (sessions) {
+		session = ao2_find(sessions, (char *) name, 0);
+		ao2_ref(sessions, -1);
+		if (session) {
+			unref_mansession(session);
+			inuse = 1;
+		}
 	}
 	return inuse;
 }
@@ -1824,6 +1838,7 @@
 /*! \brief CLI command manager list connected */
 static char *handle_showmanconn(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)
 {
+	struct ao2_container *sessions;
 	struct mansession_session *session;
 	time_t now = time(NULL);
 #define HSMCONN_FORMAT1 "  %-15.15s  %-55.55s  %-10.10s  %-10.10s  %-8.8s  %-8.8s  %-5.5s  %-5.5s\n"
@@ -1845,15 +1860,26 @@
 
 	ast_cli(a->fd, HSMCONN_FORMAT1, "Username", "IP Address", "Start", "Elapsed", "FileDes", "HttpCnt", "Read", "Write");
 
-	i = ao2_iterator_init(sessions, 0);
-	while ((session = ao2_iterator_next(&i))) {
-		ao2_lock(session);
-		ast_cli(a->fd, HSMCONN_FORMAT2, session->username, ast_sockaddr_stringify_addr(&session->addr), (int)(session->sessionstart), (int)(now - session->sessionstart), session->fd, session->inuse, session->readperm, session->writeperm);
-		count++;
-		ao2_unlock(session);
-		unref_mansession(session);
-	}
-	ao2_iterator_destroy(&i);
+	sessions = ao2_global_obj_ref(mgr_sessions);
+	if (sessions) {
+		i = ao2_iterator_init(sessions, 0);
+		ao2_ref(sessions, -1);
+		while ((session = ao2_iterator_next(&i))) {
+			ao2_lock(session);
+			ast_cli(a->fd, HSMCONN_FORMAT2, session->username,
+				ast_sockaddr_stringify_addr(&session->addr),
+				(int) (session->sessionstart),
+				(int) (now - session->sessionstart),
+				session->fd,
+				session->inuse,
+				session->readperm,
+				session->writeperm);
+			count++;
+			ao2_unlock(session);
+			unref_mansession(session);
+		}
+		ao2_iterator_destroy(&i);
+	}
 	ast_cli(a->fd, "%d users connected.\n", count);
 
 	return CLI_SUCCESS;
@@ -5486,15 +5512,17 @@
 /*! \brief remove at most n_max stale session from the list. */
 static void purge_sessions(int n_max)
 {
+	struct ao2_container *sessions;
 	struct mansession_session *session;
 	time_t now = time(NULL);
 	struct ao2_iterator i;
 
+	sessions = ao2_global_obj_ref(mgr_sessions);
 	if (!sessions) {
 		return;
 	}
-
 	i = ao2_iterator_init(sessions, 0);
+	ao2_ref(sessions, -1);
 	while ((session = ao2_iterator_next(&i)) && n_max > 0) {
 		ao2_lock(session);
 		if (session->sessiontimeout && (now > session->sessiontimeout) && !session->inuse) {
@@ -5573,9 +5601,11 @@
 AST_THREADSTORAGE(manager_event_buf);
 #define MANAGER_EVENT_BUF_INITSIZE   256
 
-int __ast_manager_event_multichan(int category, const char *event, int chancount, struct
-	ast_channel **chans, const char *file, int line, const char *func, const char *fmt, ...)
-{
+int __ast_manager_event_multichan(int category, const char *event, int chancount,
+	struct ast_channel **chans, const char *file, int line, const char *func,
+	const char *fmt, ...)
+{
+	RAII_VAR(struct ao2_container *, sessions, ao2_global_obj_ref(mgr_sessions), ao2_cleanup);
 	struct mansession_session *session;
 	struct manager_custom_hook *hook;
 	struct ast_str *auth = ast_str_alloca(80);
@@ -5859,6 +5889,7 @@
  */
 static struct mansession_session *find_session(uint32_t ident, int incinuse)
 {
+	struct ao2_container *sessions;
 	struct mansession_session *session;
 	struct ao2_iterator i;
 
@@ -5866,7 +5897,12 @@
 		return NULL;
 	}
 
+	sessions = ao2_global_obj_ref(mgr_sessions);
+	if (!sessions) {
+		return NULL;
+	}
 	i = ao2_iterator_init(sessions, 0);
+	ao2_ref(sessions, -1);
 	while ((session = ao2_iterator_next(&i))) {
 		ao2_lock(session);
 		if (session->managerid == ident && !session->needdestroy) {
@@ -5893,13 +5929,19 @@
 static struct mansession_session *find_session_by_nonce(const char *username, unsigned long nonce, int *stale)
 {
 	struct mansession_session *session;
+	struct ao2_container *sessions;
 	struct ao2_iterator i;
 
 	if (nonce == 0 || username == NULL || stale == NULL) {
 		return NULL;
 	}
 
+	sessions = ao2_global_obj_ref(mgr_sessions);
+	if (!sessions) {
+		return NULL;
+	}
 	i = ao2_iterator_init(sessions, 0);
+	ao2_ref(sessions, -1);
 	while ((session = ao2_iterator_next(&i))) {
 		ao2_lock(session);
 		if (!strcasecmp(session->username, username) && session->managerid == nonce) {
@@ -5913,6 +5955,7 @@
 		unref_mansession(session);
 	}
 	ao2_iterator_destroy(&i);
+
 	return session;
 }
 
@@ -5936,13 +5979,19 @@
 {
 	int result = 0;
 	struct mansession_session *session;
+	struct ao2_container *sessions;
 	struct ao2_iterator i;
 
 	if (ident == 0) {
 		return 0;
 	}
 
+	sessions = ao2_global_obj_ref(mgr_sessions);
+	if (!sessions) {
+		return 0;
+	}
 	i = ao2_iterator_init(sessions, 0);
+	ao2_ref(sessions, -1);
 	while ((session = ao2_iterator_next(&i))) {
 		ao2_lock(session);
 		if ((session->managerid == ident) && (session->readperm & perm)) {
@@ -5955,6 +6004,7 @@
 		unref_mansession(session);
 	}
 	ao2_iterator_destroy(&i);
+
 	return result;
 }
 
@@ -5962,13 +6012,19 @@
 {
 	int result = 0;
 	struct mansession_session *session;
+	struct ao2_container *sessions;
 	struct ao2_iterator i;
 
 	if (ident == 0) {
 		return 0;
 	}
 
+	sessions = ao2_global_obj_ref(mgr_sessions);
+	if (!sessions) {
+		return 0;
+	}
 	i = ao2_iterator_init(sessions, 0);
+	ao2_ref(sessions, -1);
 	while ((session = ao2_iterator_next(&i))) {
 		ao2_lock(session);
 		if ((session->managerid == ident) && (session->writeperm & perm)) {
@@ -5981,6 +6037,7 @@
 		unref_mansession(session);
 	}
 	ao2_iterator_destroy(&i);
+
 	return result;
 }
 
@@ -6943,7 +7000,13 @@
 
 	if (!strcasecmp(args.param, "sessions")) {
 		int no_sessions = 0;
-		ao2_callback_data(sessions, 0, get_manager_sessions_cb, /*login name*/ data, &no_sessions);
+		struct ao2_container *sessions;
+
+		sessions = ao2_global_obj_ref(mgr_sessions);
+		if (sessions) {
+			ao2_callback_data(sessions, 0, get_manager_sessions_cb, /*login name*/ data, &no_sessions);
+			ao2_ref(sessions, -1);
+		}
 		snprintf(buf, len, "%d", no_sessions);
 	} else {
 		ast_log(LOG_ERROR, "Invalid arguments provided to function AMI_CLIENT: %s\n", args.param);
@@ -7353,10 +7416,7 @@
 		ami_tls_cfg.cipher = NULL;
 	}
 
-	if (sessions) {
-		ao2_ref(sessions, -1);
-		sessions = NULL;
-	}
+	ao2_global_obj_release(mgr_sessions);
 
 	while ((user = AST_LIST_REMOVE_HEAD(&users, list))) {
 		manager_free_user(user);
@@ -7792,8 +7852,14 @@
 	AST_RWLIST_UNLOCK(&users);
 
 	if (!reload) {
+		struct ao2_container *sessions;
+
 		/* If you have a NULL hash fn, you only need a single bucket */
 		sessions = ao2_container_alloc(1, NULL, mansession_cmp_fn);
+		if (sessions) {
+			ao2_global_obj_replace_unref(mgr_sessions, sessions);
+			ao2_ref(sessions, -1);
+		}
 	}
 
 	if (webmanager_enabled && manager_enabled) {




More information about the asterisk-commits mailing list