[asterisk-commits] seanbright: branch 1.4 r199022 - in /branches/1.4: include/ main/

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Thu Jun 4 09:15:03 CDT 2009


Author: seanbright
Date: Thu Jun  4 09:14:57 2009
New Revision: 199022

URL: http://svn.asterisk.org/svn-view/asterisk?view=rev&rev=199022
Log:
Safely handle AMI connections/reload requests that occur during startup.

During asterisk startup, a lock on the list of modules is obtained by the
primary thread while each module is initialized.  Issue 13778 pointed out a
problem with this approach, however.  Because the AMI is loaded before other
modules, it is possible for a module reload to be issued by a connected client
(via Action: Command), causing a deadlock.

The resolution for 13778 was to move initialization of the manager to happen
after the other modules had already been lodaded.  While this fixed this
particular issue, it caused a problem for users (like FreePBX) who call AMI
scripts via an #exec in a configuration file (See issue 15189).

The solution I have come up with is to defer any reload requests that come in
until after the server is fully booted.  When a call comes in to
ast_module_reload (from wherever) before we are fully booted, the request is
added to a queue of pending requests.  Once we are done booting up, we then
execute these deferred requests in turn.

Note that I have tried to make this a bit more intelligent in that it will not
queue up more than 1 request for the same module to be reloaded, and if a
general reload request comes in ('module reload') the queue is flushed and we
only issue a single deferred reload for the entire system.

As for how this will impact existing installations - Before 13778, a reload
issued before module initialization was completed would result in a deadlock.
After 13778, you simply couldn't connect to the manager during startup (which
causes problems with #exec-that-calls-AMI configuration files).  I believe this
is a good general purpose solution that won't negatively impact existing
installations.

(closes issue #15189)
Reported by: p_lindheimer
Patches:
      06032009_15189_deferred_reloads.diff uploaded by seanbright (license 71)
Tested by: p_lindheimer, seanbright

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

Modified:
    branches/1.4/include/asterisk.h
    branches/1.4/main/asterisk.c
    branches/1.4/main/loader.c

Modified: branches/1.4/include/asterisk.h
URL: http://svn.asterisk.org/svn-view/asterisk/branches/1.4/include/asterisk.h?view=diff&rev=199022&r1=199021&r2=199022
==============================================================================
--- branches/1.4/include/asterisk.h (original)
+++ branches/1.4/include/asterisk.h Thu Jun  4 09:14:57 2009
@@ -126,6 +126,18 @@
 int ast_module_reload(const char *name);
 
 /*!
+ * \brief Process reload requests received during startup.
+ *
+ * This function requests that the loader execute the pending reload requests
+ * that were queued during server startup.
+ *
+ * \note This function will do nothing if the server has not completely started
+ *       up.  Once called, the reload queue is emptied, and further invocations
+ *       will have no affect.
+ */
+void ast_process_pending_reloads(void);
+
+/*!
  * \brief Register a function to be executed before Asterisk exits.
  * \param func The callback function to use.
  *

Modified: branches/1.4/main/asterisk.c
URL: http://svn.asterisk.org/svn-view/asterisk/branches/1.4/main/asterisk.c?view=diff&rev=199022&r1=199021&r2=199022
==============================================================================
--- branches/1.4/main/asterisk.c (original)
+++ branches/1.4/main/asterisk.c Thu Jun  4 09:14:57 2009
@@ -3111,6 +3111,11 @@
 
 	ast_channels_init();
 
+	if (init_manager()) {
+		printf("%s", term_quit());
+		exit(1);
+	}
+
 	if (ast_cdr_engine_init()) {
 		printf("%s", term_quit());
 		exit(1);
@@ -3156,15 +3161,6 @@
 	}
 
 	if (load_modules(0)) {
-		printf("%s", term_quit());
-		exit(1);
-	}
-
-	/* AMI is initialized after loading modules because of a potential
-	 * conflict between issuing a module reload from manager and
-	 * registering manager actions.  This will cause reversed locking
-	 * order between the module list and manager actions list. */
-	if (init_manager()) {
 		printf("%s", term_quit());
 		exit(1);
 	}
@@ -3184,6 +3180,9 @@
 		sig_alert_pipe[0] = sig_alert_pipe[1] = -1;
 
 	ast_set_flag(&ast_options, AST_OPT_FLAG_FULLY_BOOTED);
+
+	ast_process_pending_reloads();
+
 	pthread_sigmask(SIG_UNBLOCK, &sigs, NULL);
 
 #ifdef __AST_DEBUG_MALLOC

Modified: branches/1.4/main/loader.c
URL: http://svn.asterisk.org/svn-view/asterisk/branches/1.4/main/loader.c?view=diff&rev=199022&r1=199021&r2=199022
==============================================================================
--- branches/1.4/main/loader.c (original)
+++ branches/1.4/main/loader.c Thu Jun  4 09:14:57 2009
@@ -102,6 +102,15 @@
 
 AST_MUTEX_DEFINE_STATIC(reloadlock);
 
+struct reload_queue_item {
+	AST_LIST_ENTRY(reload_queue_item) entry;
+	char module[0];
+};
+
+static int do_full_reload = 0;
+
+static AST_LIST_HEAD_STATIC(reload_queue, reload_queue_item);
+
 /* when dynamic modules are being loaded, ast_module_register() will
    need to know what filename the module was loaded from while it
    is being registered
@@ -529,11 +538,83 @@
 	return ret;
 }
 
+void ast_process_pending_reloads(void)
+{
+	struct reload_queue_item *item;
+
+	if (!ast_fully_booted) {
+		return;
+	}
+
+	AST_LIST_LOCK(&reload_queue);
+
+	if (do_full_reload) {
+		do_full_reload = 0;
+		AST_LIST_UNLOCK(&reload_queue);
+		ast_log(LOG_NOTICE, "Executing deferred reload request.\n");
+		ast_module_reload(NULL);
+		return;
+	}
+
+	while ((item = AST_LIST_REMOVE_HEAD(&reload_queue, entry))) {
+		ast_log(LOG_NOTICE, "Executing deferred reload request for module '%s'.\n", item->module);
+		ast_module_reload(item->module);
+		ast_free(item);
+	}
+
+	AST_LIST_UNLOCK(&reload_queue);
+}
+
+static void queue_reload_request(const char *module)
+{
+	struct reload_queue_item *item;
+
+	AST_LIST_LOCK(&reload_queue);
+
+	if (do_full_reload) {
+		AST_LIST_UNLOCK(&reload_queue);
+		return;
+	}
+
+	if (ast_strlen_zero(module)) {
+		/* A full reload request (when module is NULL) wipes out any previous
+		   reload requests and causes the queue to ignore future ones */
+		while ((item = AST_LIST_REMOVE_HEAD(&reload_queue, entry))) {
+			ast_free(item);
+		}
+		do_full_reload = 1;
+	} else {
+		/* No reason to add the same module twice */
+		AST_LIST_TRAVERSE(&reload_queue, item, entry) {
+			if (!strcasecmp(item->module, module)) {
+				AST_LIST_UNLOCK(&reload_queue);
+				return;
+			}
+		}
+		item = ast_calloc(1, sizeof(*item) + strlen(module) + 1);
+		if (!item) {
+			ast_log(LOG_ERROR, "Failed to allocate reload queue item.\n");
+			AST_LIST_UNLOCK(&reload_queue);
+			return;
+		}
+		strcpy(item->module, module);
+		AST_LIST_INSERT_TAIL(&reload_queue, item, entry);
+	}
+	AST_LIST_UNLOCK(&reload_queue);
+}
+
 int ast_module_reload(const char *name)
 {
 	struct ast_module *cur;
 	int res = 0; /* return value. 0 = not found, others, see below */
 	int i;
+
+	/* If we aren't fully booted, we just pretend we reloaded but we queue this
+	   up to run once we are booted up. */
+	if (!ast_fully_booted) {
+		queue_reload_request(name);
+		return 0;
+	}
 
 	if (ast_mutex_trylock(&reloadlock)) {
 		ast_verbose("The previous reload command didn't finish yet\n");




More information about the asterisk-commits mailing list