[Asterisk-code-review] res stasis: Add control ref to playback and recording structs. (asterisk[13])
Anonymous Coward
asteriskteam at digium.com
Thu Mar 31 13:39:04 CDT 2016
Anonymous Coward #1000019 has submitted this change and it was merged.
Change subject: res_stasis: Add control ref to playback and recording structs.
......................................................................
res_stasis: Add control ref to playback and recording structs.
The stasis_app_playback and stasis_app_recording structs need to have a
struct stasis_app_control ref. Other threads can get a reference to the
playback and recording structs from their respective global container.
These other threads can then use the control pointer they contain after
the control struct has gone.
* Add control ref to stasis_app_playback and stasis_app_recording structs.
With the refs added, the control command queue can now have a circular
control reference which will cause the control struct to never get
released if the control's command queue is not flushed when the channel
leaves the Stasis application. Also the command queue needs better
protection from adding commands if the control->is_done flag is set.
* Flush the control command queue on exit.
ASTERISK-25882 #close
Change-Id: I3cf1fb59cbe6f50f20d9e35a2c07ac07d7f4320d
---
M include/asterisk/stasis_app.h
M res/ari/resource_bridges.c
M res/res_stasis.c
M res/res_stasis_playback.c
M res/res_stasis_recording.c
M res/stasis/control.c
M res/stasis/control.h
7 files changed, 62 insertions(+), 10 deletions(-)
Approvals:
Mark Michelson: Looks good to me, approved
Anonymous Coward #1000019: Verified
Joshua Colp: Looks good to me, but someone else must approve
diff --git a/include/asterisk/stasis_app.h b/include/asterisk/stasis_app.h
index f2b07e0..90ef82e 100644
--- a/include/asterisk/stasis_app.h
+++ b/include/asterisk/stasis_app.h
@@ -440,6 +440,16 @@
struct stasis_app_control *control);
/*!
+ * \brief Flush the control command queue.
+ * \since 13.9.0
+ *
+ * \param control Control object to flush command queue.
+ *
+ * \return Nothing
+ */
+void stasis_app_control_flush_queue(struct stasis_app_control *control);
+
+/*!
* \brief Returns the uniqueid of the channel associated with this control
*
* \param control Control object.
diff --git a/res/ari/resource_bridges.c b/res/ari/resource_bridges.c
index ad37053..7861f4a 100644
--- a/res/ari/resource_bridges.c
+++ b/res/ari/resource_bridges.c
@@ -297,10 +297,11 @@
thread_data = NULL;
stasis_app_control_execute_until_exhausted(bridge_channel, control);
+ stasis_app_control_flush_queue(control);
- ast_hangup(bridge_channel);
- ao2_cleanup(control);
stasis_forward_cancel(forward);
+ ao2_cleanup(control);
+ ast_hangup(bridge_channel);
return NULL;
}
diff --git a/res/res_stasis.c b/res/res_stasis.c
index cecc6ef..4bb967d 100644
--- a/res/res_stasis.c
+++ b/res/res_stasis.c
@@ -1179,6 +1179,11 @@
return control_is_done(control);
}
+void stasis_app_control_flush_queue(struct stasis_app_control *control)
+{
+ control_flush_queue(control);
+}
+
struct ast_datastore_info set_end_published_info = {
.type = "stasis_end_published",
};
@@ -1371,6 +1376,8 @@
remove_stasis_end_published(chan);
}
+ control_flush_queue(control);
+
/* Stop any lingering silence generator */
control_silence_stop_now(control);
diff --git a/res/res_stasis_playback.c b/res/res_stasis_playback.c
index 779dd77..7fdb5d4 100644
--- a/res/res_stasis_playback.c
+++ b/res/res_stasis_playback.c
@@ -118,6 +118,7 @@
{
struct stasis_app_playback *playback = obj;
+ ao2_cleanup(playback->control);
ast_string_field_free_memory(playback);
}
@@ -143,6 +144,7 @@
ast_string_field_set(playback, id, uuid);
}
+ ao2_ref(control, +1);
playback->control = control;
ao2_ref(playback, +1);
diff --git a/res/res_stasis_recording.c b/res/res_stasis_recording.c
index 392d92c..f907b7a 100644
--- a/res/res_stasis_recording.c
+++ b/res/res_stasis_recording.c
@@ -358,6 +358,7 @@
struct stasis_app_recording *recording = obj;
ast_free(recording->absolute_name);
+ ao2_cleanup(recording->control);
ao2_cleanup(recording->options);
}
@@ -413,6 +414,7 @@
ao2_ref(options, +1);
recording->options = options;
+ ao2_ref(control, +1);
recording->control = control;
recording->state = STASIS_APP_RECORDING_STATE_QUEUED;
diff --git a/res/stasis/control.c b/res/stasis/control.c
index 00385a0..97b0b88 100644
--- a/res/stasis/control.c
+++ b/res/stasis/control.c
@@ -251,6 +251,11 @@
}
ao2_lock(control->command_queue);
+ if (control->is_done) {
+ ao2_unlock(control->command_queue);
+ ao2_ref(command, -1);
+ return NULL;
+ }
if (can_exec_fn && (retval = can_exec_fn(control))) {
ao2_unlock(control->command_queue);
command_complete(command, retval);
@@ -402,7 +407,10 @@
void control_mark_done(struct stasis_app_control *control)
{
+ /* Locking necessary to sync with other threads adding commands to the queue. */
+ ao2_lock(control->command_queue);
control->is_done = 1;
+ ao2_unlock(control->command_queue);
}
struct stasis_app_control_continue_data {
@@ -427,7 +435,7 @@
/* Called from stasis_app_exec thread; no lock needed */
ast_explicit_goto(control->channel, continue_data->context, continue_data->extension, continue_data->priority);
- control->is_done = 1;
+ control_mark_done(control);
return 0;
}
@@ -1115,24 +1123,36 @@
return ast_queue_control(control->channel, frame_type);
}
+void control_flush_queue(struct stasis_app_control *control)
+{
+ struct ao2_iterator iter;
+ struct stasis_app_command *command;
+
+ iter = ao2_iterator_init(control->command_queue, AO2_ITERATOR_UNLINK);
+ while ((command = ao2_iterator_next(&iter))) {
+ command_complete(command, -1);
+ ao2_ref(command, -1);
+ }
+ ao2_iterator_destroy(&iter);
+}
+
int control_dispatch_all(struct stasis_app_control *control,
struct ast_channel *chan)
{
int count = 0;
- struct ao2_iterator i;
- void *obj;
+ struct ao2_iterator iter;
+ struct stasis_app_command *command;
ast_assert(control->channel == chan);
- i = ao2_iterator_init(control->command_queue, AO2_ITERATOR_UNLINK);
-
- while ((obj = ao2_iterator_next(&i))) {
- RAII_VAR(struct stasis_app_command *, command, obj, ao2_cleanup);
+ iter = ao2_iterator_init(control->command_queue, AO2_ITERATOR_UNLINK);
+ while ((command = ao2_iterator_next(&iter))) {
command_invoke(command, control, chan);
+ ao2_ref(command, -1);
++count;
}
+ ao2_iterator_destroy(&iter);
- ao2_iterator_destroy(&i);
return count;
}
diff --git a/res/stasis/control.h b/res/stasis/control.h
index d053a35..1d37a49 100644
--- a/res/stasis/control.h
+++ b/res/stasis/control.h
@@ -41,6 +41,16 @@
struct stasis_app_control *control_create(struct ast_channel *channel, struct stasis_app *app);
/*!
+ * \brief Flush the control command queue.
+ * \since 13.9.0
+ *
+ * \param control Control object to flush command queue.
+ *
+ * \return Nothing
+ */
+void control_flush_queue(struct stasis_app_control *control);
+
+/*!
* \brief Dispatch all commands enqueued to this control.
*
* \param control Control object to dispatch.
--
To view, visit https://gerrit.asterisk.org/2509
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I3cf1fb59cbe6f50f20d9e35a2c07ac07d7f4320d
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
More information about the asterisk-code-review
mailing list