[svn-commits] dlee: trunk r395120 - in /trunk/res: res_stasis.c stasis/app.c stasis/app.h

SVN commits to the Digium repositories svn-commits at lists.digium.com
Tue Jul 23 08:42:51 CDT 2013


Author: dlee
Date: Tue Jul 23 08:42:46 2013
New Revision: 395120

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=395120
Log:
Continue events when ARI WebSocket reconnects

This patch addresses a bug in the /ari/events WebSocket in handling
reconnects.

When a Stasis application's associated WebSocket was disconnected and
reconnected, it would not receive events for any channels or bridges
it was subscribed to.

The fix was to lazily clean up Stasis application registrations,
instead of removing them as soon as the WebSocket goes away.

When an application is unregistered at the WebSocket level, the
underlying application is simply deactivated. If the application
WebSocket is reconnected, the application is reactivated for the new
connection.

To avoid memory leaks from lingering, unused application, the
application list is cleaned up whenever new applications are
registered/unregistered.

(closes issue ASTERISK-21970)
Review: https://reviewboard.asterisk.org/r/2678/

Modified:
    trunk/res/res_stasis.c
    trunk/res/stasis/app.c
    trunk/res/stasis/app.h

Modified: trunk/res/res_stasis.c
URL: http://svnview.digium.com/svn/asterisk/trunk/res/res_stasis.c?view=diff&rev=395120&r1=395119&r2=395120
==============================================================================
--- trunk/res/res_stasis.c (original)
+++ trunk/res/res_stasis.c Tue Jul 23 08:42:46 2013
@@ -564,6 +564,11 @@
 			"Stasis app '%s' not registered\n", app_name);
 		return -1;
 	}
+	if (!app_is_active(app)) {
+		ast_log(LOG_ERROR,
+			"Stasis app '%s' not active\n", app_name);
+		return -1;
+	}
 
 	control = control_create(chan);
 	if (!control) {
@@ -575,7 +580,7 @@
 	res = app_send_start_msg(app, chan, argc, argv);
 	if (res != 0) {
 		ast_log(LOG_ERROR,
-			"Error sending start message to %s\n", app_name);
+			"Error sending start message to '%s'\n", app_name);
 		return res;
 	}
 
@@ -662,6 +667,29 @@
 	return 0;
 }
 
+static int cleanup_cb(void *obj, void *arg, int flags)
+{
+	struct app *app = obj;
+
+	if (!app_is_finished(app)) {
+		return 0;
+	}
+
+	ast_verb(1, "Cleaning up application '%s'\n", app_name(app));
+
+	return CMP_MATCH;
+
+}
+
+/*!
+ * \brief Clean up any old apps that we don't need any more.
+ */
+static void cleanup(void)
+{
+	ao2_callback(apps_registry, OBJ_MULTIPLE | OBJ_NODATA | OBJ_UNLINK,
+		cleanup_cb, NULL);
+}
+
 int stasis_app_register(const char *app_name, stasis_app_cb handler, void *data)
 {
 	RAII_VAR(struct app *, app, NULL, ao2_cleanup);
@@ -671,15 +699,6 @@
 	app = ao2_find(apps_registry, app_name, OBJ_KEY | OBJ_NOLOCK);
 
 	if (app) {
-		RAII_VAR(struct ast_json *, msg, NULL, ast_json_unref);
-
-		msg = ast_json_pack("{s: s, s: s}",
-			"type", "ApplicationReplaced",
-			"application", app_name);
-		if (msg) {
-			app_send(app, msg);
-		}
-
 		app_update(app, handler, data);
 	} else {
 		app = app_create(app_name, handler, data);
@@ -690,15 +709,34 @@
 		}
 	}
 
+	/* We lazily clean up the apps_registry, because it's good enough to
+	 * prevent memory leaks, and we're lazy.
+	 */
+	cleanup();
 	return 0;
 }
 
 void stasis_app_unregister(const char *app_name)
 {
-	if (app_name) {
-		ao2_cleanup(ao2_find(
-				apps_registry, app_name, OBJ_KEY | OBJ_UNLINK));
-	}
+	RAII_VAR(struct app *, app, NULL, ao2_cleanup);
+
+	if (!app_name) {
+		return;
+	}
+
+	app = ao2_find(apps_registry, app_name, OBJ_KEY);
+	if (!app) {
+		ast_log(LOG_ERROR,
+			"Stasis app '%s' not registered\n", app_name);
+		return;
+	}
+
+	app_deactivate(app);
+
+	/* There's a decent chance that app is ready for cleanup. Go ahead
+	 * and clean up, just in case
+	 */
+	cleanup();
 }
 
 void stasis_app_ref(void)

Modified: trunk/res/stasis/app.c
URL: http://svnview.digium.com/svn/asterisk/trunk/res/stasis/app.c?view=diff&rev=395120&r1=395119&r2=395120
==============================================================================
--- trunk/res/stasis/app.c (original)
+++ trunk/res/stasis/app.c Tue Jul 23 08:42:46 2013
@@ -77,6 +77,8 @@
 	ast_assert(name != NULL);
 	ast_assert(handler != NULL);
 
+	ast_verb(1, "Creating Stasis app '%s'\n", name);
+
 	size = sizeof(*app) + strlen(name) + 1;
 	app = ao2_alloc_options(size, app_dtor, AO2_ALLOC_OPT_LOCK_MUTEX);
 
@@ -105,9 +107,16 @@
 
 int app_add_channel(struct app *app, const struct ast_channel *chan)
 {
+	SCOPED_AO2LOCK(lock, app);
 	const char *uniqueid;
+
+	ast_assert(app != NULL);
 	ast_assert(chan != NULL);
-	ast_assert(app != NULL);
+
+	/* Don't accept new channels in an inactive application */
+	if (!app->handler) {
+		return -1;
+	}
 
 	uniqueid = ast_channel_uniqueid(chan);
 	return ast_str_container_add(app->channels, uniqueid) ? -1 : 0;
@@ -115,24 +124,35 @@
 
 void app_remove_channel(struct app* app, const struct ast_channel *chan)
 {
+	SCOPED_AO2LOCK(lock, app);
+
+	ast_assert(app != NULL);
 	ast_assert(chan != NULL);
-	ast_assert(app != NULL);
 
 	ao2_find(app->channels, ast_channel_uniqueid(chan), OBJ_KEY | OBJ_NODATA | OBJ_UNLINK);
 }
 
 int app_add_bridge(struct app *app, const char *uniqueid)
 {
+	SCOPED_AO2LOCK(lock, app);
+
+	ast_assert(app != NULL);
 	ast_assert(uniqueid != NULL);
-	ast_assert(app != NULL);
+
+	/* Don't accept new bridges in an inactive application */
+	if (!app->handler) {
+		return -1;
+	}
 
 	return ast_str_container_add(app->bridges, uniqueid) ? -1 : 0;
 }
 
 void app_remove_bridge(struct app* app, const char *uniqueid)
 {
+	SCOPED_AO2LOCK(lock, app);
+
+	ast_assert(app != NULL);
 	ast_assert(uniqueid != NULL);
-	ast_assert(app != NULL);
 
 	ao2_find(app->bridges, uniqueid, OBJ_KEY | OBJ_NODATA | OBJ_UNLINK | OBJ_MULTIPLE);
 }
@@ -144,16 +164,77 @@
  */
 void app_send(struct app *app, struct ast_json *message)
 {
-	app->handler(app->data, app->name, message);
+	stasis_app_cb handler;
+	RAII_VAR(void *, data, NULL, ao2_cleanup);
+
+	/* Copy off mutable state with lock held */
+	{
+		SCOPED_AO2LOCK(lock, app);
+		handler = app->handler;
+		if (app->data) {
+			ao2_ref(app->data, +1);
+			data = app->data;
+		}
+		/* Name is immutable; no need to copy */
+	}
+
+	if (!handler) {
+		ast_verb(3,
+			"Inactive Stasis app '%s' missed message\n", app->name);
+		return;
+	}
+
+	handler(data, app->name, message);
+}
+
+void app_deactivate(struct app *app)
+{
+	SCOPED_AO2LOCK(lock, app);
+	ast_verb(1, "Deactivating Stasis app '%s'\n", app->name);
+	app->handler = NULL;
+	ao2_cleanup(app->data);
+	app->data = NULL;
+}
+
+int app_is_active(struct app *app)
+{
+	SCOPED_AO2LOCK(lock, app);
+	return app->handler != NULL;
+}
+
+int app_is_finished(struct app *app)
+{
+	SCOPED_AO2LOCK(lock, app);
+
+	return app->handler == NULL &&
+		ao2_container_count(app->channels) == 0;
 }
 
 void app_update(struct app *app, stasis_app_cb handler, void *data)
 {
 	SCOPED_AO2LOCK(lock, app);
+
+	if (app->handler) {
+		RAII_VAR(struct ast_json *, msg, NULL, ast_json_unref);
+
+		ast_verb(1, "Replacing Stasis app '%s'\n", app->name);
+
+		msg = ast_json_pack("{s: s, s: s}",
+			"type", "ApplicationReplaced",
+			"application", app_name);
+		if (msg) {
+			app_send(app, msg);
+		}
+	} else {
+		ast_verb(1, "Activating Stasis app '%s'\n", app->name);
+	}
+
 
 	app->handler = handler;
 	ao2_cleanup(app->data);
-	ao2_ref(data, +1);
+	if (data) {
+		ao2_ref(data, +1);
+	}
 	app->data = data;
 }
 

Modified: trunk/res/stasis/app.h
URL: http://svnview.digium.com/svn/asterisk/trunk/res/stasis/app.h?view=diff&rev=395120&r1=395119&r2=395120
==============================================================================
--- trunk/res/stasis/app.h (original)
+++ trunk/res/stasis/app.h Tue Jul 23 08:42:46 2013
@@ -48,7 +48,37 @@
 struct app *app_create(const char *name, stasis_app_cb handler, void *data);
 
 /*!
+ * \brief Deactivates an application.
+ *
+ * Any channels currently in the application remain active (since the app might
+ * come back), but new channels are rejected.
+ *
+ * \param app Application to deactivate.
+ */
+void app_deactivate(struct app *app);
+
+/*!
+ * \brief Checks whether an app is active.
+ *
+ * \param app Application to check.
+ * \return True (non-zero) if app is active.
+ * \return False (zero) if app has been deactivated.
+ */
+int app_is_active(struct app *app);
+
+/*!
+ * \brief Checks whether a deactivated app has no channels.
+ *
+ * \param app Application to check.
+ * \param True (non-zero) if app is deactivated, and has no associated channels.
+ * \param False (zero) otherwise.
+ */
+int app_is_finished(struct app *app);
+
+/*!
  * \brief Update the handler and data for a \c res_stasis application.
+ *
+ * If app has been deactivated, this will reactivate it.
  *
  * \param app Application to update.
  * \param handler New application callback.




More information about the svn-commits mailing list