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/


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

3 files changed, 168 insertions, 19 deletions

diff --git a/res/res_stasis.c b/res/res_stasis.c
index a294db915..347118b06 100644
--- a/res/res_stasis.c
+++ b/res/res_stasis.c
@@ -564,6 +564,11 @@ int stasis_app_exec(struct ast_channel *chan, const char *app_name, int argc,
 			"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 @@ int stasis_app_exec(struct ast_channel *chan, const char *app_name, int argc,
 	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 @@ int stasis_app_send(const char *app_name, struct ast_json *message)
 	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 @@ int stasis_app_register(const char *app_name, stasis_app_cb handler, void *data)
 	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 @@ int stasis_app_register(const char *app_name, stasis_app_cb handler, void *data)
 		}
 	}
 
+	/* 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)
diff --git a/res/stasis/app.c b/res/stasis/app.c
index 31e15c221..dfb6df338 100644
--- a/res/stasis/app.c
+++ b/res/stasis/app.c
@@ -77,6 +77,8 @@ struct app *app_create(const char *name, stasis_app_cb handler, void *data)
 	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 @@ struct app *app_create(const char *name, stasis_app_cb handler, void *data)
 
 int app_add_channel(struct app *app, const struct ast_channel *chan)
 {
+	SCOPED_AO2LOCK(lock, app);
 	const char *uniqueid;
-	ast_assert(chan != NULL);
+
 	ast_assert(app != NULL);
+	ast_assert(chan != 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 @@ int app_add_channel(struct app *app, const struct ast_channel *chan)
 
 void app_remove_channel(struct app* app, const struct ast_channel *chan)
 {
-	ast_assert(chan != NULL);
+	SCOPED_AO2LOCK(lock, app);
+
 	ast_assert(app != NULL);
+	ast_assert(chan != 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)
 {
-	ast_assert(uniqueid != NULL);
+	SCOPED_AO2LOCK(lock, app);
+
 	ast_assert(app != NULL);
+	ast_assert(uniqueid != 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)
 {
-	ast_assert(uniqueid != NULL);
+	SCOPED_AO2LOCK(lock, app);
+
 	ast_assert(app != NULL);
+	ast_assert(uniqueid != NULL);
 
 	ao2_find(app->bridges, uniqueid, OBJ_KEY | OBJ_NODATA | OBJ_UNLINK | OBJ_MULTIPLE);
 }
@@ -144,16 +164,77 @@ void app_remove_bridge(struct app* app, const char *uniqueid)
  */
 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;
 }
 
diff --git a/res/stasis/app.h b/res/stasis/app.h
index 7a5405a89..0cf92217f 100644
--- a/res/stasis/app.h
+++ b/res/stasis/app.h
@@ -48,8 +48,38 @@ struct app;
 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.
  * \param data New data pointer for the callback.