[Asterisk-code-review] res stasis: Fix crash on a hanging up channel. (asterisk[13])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged.

Change subject: res_stasis: Fix crash on a hanging up channel.
......................................................................


res_stasis: Fix crash on a hanging up channel.

* Give the struct stasis_app_control ao2 object a ref to the channel held
in the object.  Now the channel will still be around if a thread needs to
post a stasis message instead of crash because the topic was destroyed.

* Moved stopping any lingering silence generator out of the struct
stasis_app_control destructor and made it a part of exiting the Stasis
application.  Who knows which thread the destructor will be called under
so it cannot affect the channel's silence generator.  Not only was the
channel unprotected when the silence generator was stopped, stasis may no
longer even control the channel.

ASTERISK-25882

Change-Id: I21728161b5fe638cef7976fa36a605043a7497e4
---
M res/res_stasis.c
M res/stasis/control.c
M res/stasis/control.h
3 files changed, 33 insertions(+), 19 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, approved; Verified
  George Joseph: Looks good to me, but someone else must approve



diff --git a/res/res_stasis.c b/res/res_stasis.c
index 94b037e..cecc6ef 100644
--- a/res/res_stasis.c
+++ b/res/res_stasis.c
@@ -1371,6 +1371,9 @@
 		remove_stasis_end_published(chan);
 	}
 
+	/* Stop any lingering silence generator */
+	control_silence_stop_now(control);
+
 	/* There's an off chance that app is ready for cleanup. Go ahead
 	 * and clean up, just in case
 	 */
diff --git a/res/stasis/control.c b/res/stasis/control.c
index ebb7e01..00385a0 100644
--- a/res/stasis/control.c
+++ b/res/stasis/control.c
@@ -87,21 +87,19 @@
 {
 	struct stasis_app_control *control = obj;
 
+	ao2_cleanup(control->command_queue);
+
+	ast_channel_cleanup(control->channel);
+	ao2_cleanup(control->app);
+
+	ast_cond_destroy(&control->wait_cond);
 	AST_LIST_HEAD_DESTROY(&control->add_rules);
 	AST_LIST_HEAD_DESTROY(&control->remove_rules);
-
-	/* We may have a lingering silence generator; free it */
-	ast_channel_stop_silence_generator(control->channel, control->silgen);
-	control->silgen = NULL;
-
-	ao2_cleanup(control->command_queue);
-	ast_cond_destroy(&control->wait_cond);
-	ao2_cleanup(control->app);
 }
 
 struct stasis_app_control *control_create(struct ast_channel *channel, struct stasis_app *app)
 {
-	RAII_VAR(struct stasis_app_control *, control, NULL, ao2_cleanup);
+	struct stasis_app_control *control;
 	int res;
 
 	control = ao2_alloc(sizeof(*control), control_dtor);
@@ -109,28 +107,29 @@
 		return NULL;
 	}
 
-	control->app = ao2_bump(app);
+	AST_LIST_HEAD_INIT(&control->add_rules);
+	AST_LIST_HEAD_INIT(&control->remove_rules);
 
 	res = ast_cond_init(&control->wait_cond, NULL);
 	if (res != 0) {
 		ast_log(LOG_ERROR, "Error initializing ast_cond_t: %s\n",
 			strerror(errno));
+		ao2_ref(control, -1);
 		return NULL;
 	}
+
+	control->app = ao2_bump(app);
+
+	ast_channel_ref(channel);
+	control->channel = channel;
 
 	control->command_queue = ao2_container_alloc_list(
 		AO2_ALLOC_OPT_LOCK_MUTEX, 0, NULL, NULL);
-
 	if (!control->command_queue) {
+		ao2_ref(control, -1);
 		return NULL;
 	}
 
-	control->channel = channel;
-
-	AST_LIST_HEAD_INIT(&control->add_rules);
-	AST_LIST_HEAD_INIT(&control->remove_rules);
-
-	ao2_ref(control, +1);
 	return control;
 }
 
@@ -785,8 +784,7 @@
 	stasis_app_send_command_async(control, app_control_silence_start, NULL, NULL);
 }
 
-static int app_control_silence_stop(struct stasis_app_control *control,
-	struct ast_channel *chan, void *data)
+void control_silence_stop_now(struct stasis_app_control *control)
 {
 	if (control->silgen) {
 		ast_debug(3, "%s: Stopping silence generator\n",
@@ -795,7 +793,12 @@
 			control->channel, control->silgen);
 		control->silgen = NULL;
 	}
+}
 
+static int app_control_silence_stop(struct stasis_app_control *control,
+	struct ast_channel *chan, void *data)
+{
+	control_silence_stop_now(control);
 	return 0;
 }
 
diff --git a/res/stasis/control.h b/res/stasis/control.h
index a139f82..d053a35 100644
--- a/res/stasis/control.h
+++ b/res/stasis/control.h
@@ -108,5 +108,13 @@
 	struct stasis_app_control *control,
 	struct ast_channel *chan, void *obj);
 
+/*!
+ * \brief Stop playing silence to a channel right now.
+ * \since 13.9.0
+ *
+ * \param control The control for chan
+ */
+void control_silence_stop_now(struct stasis_app_control *control);
+
 
 #endif /* _ASTERISK_RES_STASIS_CONTROL_H */

-- 
To view, visit https://gerrit.asterisk.org/2508
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I21728161b5fe638cef7976fa36a605043a7497e4
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>