[pulseaudio-discuss] [PATCH 06/15] sink-input, source-output: Protect unlinking against recursion
Tanu Kaskinen
tanu.kaskinen at linux.intel.com
Thu Feb 13 19:35:51 CET 2014
It's not entirely impossible that there would some day be a code path
that would cause recursive calls to pa_sink_input_unlink() or
pa_source_output_unlink() (I know this because I once created such
code path). If a recursive call happens, the unlinking functions
should notice that and return early.
This also makes it easier to be confident that the unlinking code
doesn't do anything stupid if the function happens to be called twice
for the same object (recursively or not).
Since the input/output state variable is now updated earlier than
before, update_n_corked() needed some changes too, because it can't
get the "old" state from i->state any more.
---
src/pulsecore/sink-input.c | 30 ++++++++++++++++++++----------
src/pulsecore/source-output.c | 26 ++++++++++++++++++--------
2 files changed, 38 insertions(+), 18 deletions(-)
diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c
index 3f8af98..450f5d3 100644
--- a/src/pulsecore/sink-input.c
+++ b/src/pulsecore/sink-input.c
@@ -620,16 +620,16 @@ fail:
}
/* Called from main context */
-static void update_n_corked(pa_sink_input *i, pa_sink_input_state_t state) {
+static void update_n_corked(pa_sink_input *i, pa_sink_input_state_t old_state, pa_sink_input_state_t new_state) {
pa_assert(i);
pa_assert_ctl_context();
if (!i->sink)
return;
- if (i->state == PA_SINK_INPUT_CORKED && state != PA_SINK_INPUT_CORKED)
+ if (old_state == PA_SINK_INPUT_CORKED && new_state != PA_SINK_INPUT_CORKED)
pa_assert_se(i->sink->n_corked -- >= 1);
- else if (i->state != PA_SINK_INPUT_CORKED && state == PA_SINK_INPUT_CORKED)
+ else if (old_state != PA_SINK_INPUT_CORKED && new_state == PA_SINK_INPUT_CORKED)
i->sink->n_corked++;
}
@@ -654,15 +654,15 @@ static void sink_input_set_state(pa_sink_input *i, pa_sink_input_state_t state)
pa_assert_se(pa_asyncmsgq_send(i->sink->asyncmsgq, PA_MSGOBJECT(i), PA_SINK_INPUT_MESSAGE_SET_STATE, PA_UINT_TO_PTR(state), 0, NULL) == 0);
- update_n_corked(i, state);
+ update_n_corked(i, i->state, state);
i->state = state;
for (ssync = i->sync_prev; ssync; ssync = ssync->sync_prev) {
- update_n_corked(ssync, state);
+ update_n_corked(ssync, ssync->state, state);
ssync->state = state;
}
for (ssync = i->sync_next; ssync; ssync = ssync->sync_next) {
- update_n_corked(ssync, state);
+ update_n_corked(ssync, ssync->state, state);
ssync->state = state;
}
@@ -684,6 +684,7 @@ static void sink_input_set_state(pa_sink_input *i, pa_sink_input_state_t state)
/* Called from main context */
void pa_sink_input_unlink(pa_sink_input *i) {
+ pa_sink_input_state_t old_state;
bool linked;
pa_source_output *o, *p = NULL;
@@ -693,9 +694,19 @@ void pa_sink_input_unlink(pa_sink_input *i) {
/* See pa_sink_unlink() for a couple of comments how this function
* works */
+ if (i->state == PA_SINK_INPUT_UNLINKED) {
+ pa_log_debug("Unlinking sink input %u (already unlinked, this is a no-op).", i->index);
+ return;
+ }
+
+ old_state = i->state;
+ i->state = PA_SINK_INPUT_UNLINKED;
+
+ pa_log_debug("Unlinking sink input %u.", i->index);
+
pa_sink_input_ref(i);
- linked = PA_SINK_INPUT_IS_LINKED(i->state);
+ linked = PA_SINK_INPUT_IS_LINKED(old_state);
if (linked)
pa_hook_fire(&i->core->hooks[PA_CORE_HOOK_SINK_INPUT_UNLINK], i);
@@ -725,8 +736,7 @@ void pa_sink_input_unlink(pa_sink_input *i) {
p = o;
}
- update_n_corked(i, PA_SINK_INPUT_UNLINKED);
- i->state = PA_SINK_INPUT_UNLINKED;
+ update_n_corked(i, old_state, PA_SINK_INPUT_UNLINKED);
if (linked && i->sink) {
if (pa_sink_input_is_passthrough(i))
@@ -828,7 +838,7 @@ void pa_sink_input_put(pa_sink_input *i) {
state = i->flags & PA_SINK_INPUT_START_CORKED ? PA_SINK_INPUT_CORKED : PA_SINK_INPUT_RUNNING;
- update_n_corked(i, state);
+ update_n_corked(i, i->state, state);
i->state = state;
/* We might need to update the sink's volume if we are in flat volume mode. */
diff --git a/src/pulsecore/source-output.c b/src/pulsecore/source-output.c
index 177c180..37a4752 100644
--- a/src/pulsecore/source-output.c
+++ b/src/pulsecore/source-output.c
@@ -540,16 +540,16 @@ fail:
}
/* Called from main context */
-static void update_n_corked(pa_source_output *o, pa_source_output_state_t state) {
+static void update_n_corked(pa_source_output *o, pa_source_output_state_t old_state, pa_source_output_state_t new_state) {
pa_assert(o);
pa_assert_ctl_context();
if (!o->source)
return;
- if (o->state == PA_SOURCE_OUTPUT_CORKED && state != PA_SOURCE_OUTPUT_CORKED)
+ if (old_state == PA_SOURCE_OUTPUT_CORKED && new_state != PA_SOURCE_OUTPUT_CORKED)
pa_assert_se(o->source->n_corked -- >= 1);
- else if (o->state != PA_SOURCE_OUTPUT_CORKED && state == PA_SOURCE_OUTPUT_CORKED)
+ else if (old_state != PA_SOURCE_OUTPUT_CORKED && new_state == PA_SOURCE_OUTPUT_CORKED)
o->source->n_corked++;
}
@@ -571,7 +571,7 @@ static void source_output_set_state(pa_source_output *o, pa_source_output_state_
pa_assert_se(pa_asyncmsgq_send(o->source->asyncmsgq, PA_MSGOBJECT(o), PA_SOURCE_OUTPUT_MESSAGE_SET_STATE, PA_UINT_TO_PTR(state), 0, NULL) == 0);
- update_n_corked(o, state);
+ update_n_corked(o, o->state, state);
o->state = state;
if (state != PA_SOURCE_OUTPUT_UNLINKED) {
@@ -586,6 +586,7 @@ static void source_output_set_state(pa_source_output *o, pa_source_output_state_
/* Called from main context */
void pa_source_output_unlink(pa_source_output*o) {
+ pa_source_output_state_t old_state;
bool linked;
pa_assert(o);
pa_assert_ctl_context();
@@ -593,9 +594,19 @@ void pa_source_output_unlink(pa_source_output*o) {
/* See pa_sink_unlink() for a couple of comments how this function
* works */
+ if (o->state == PA_SOURCE_OUTPUT_UNLINKED) {
+ pa_log_debug("Unlinking source output %u (already unlinked, this is a no-op).", o->index);
+ return;
+ }
+
+ old_state = o->state;
+ o->state = PA_SOURCE_OUTPUT_UNLINKED;
+
+ pa_log_debug("Unlinking source output %u.", o->index);
+
pa_source_output_ref(o);
- linked = PA_SOURCE_OUTPUT_IS_LINKED(o->state);
+ linked = PA_SOURCE_OUTPUT_IS_LINKED(old_state);
if (linked)
pa_hook_fire(&o->core->hooks[PA_CORE_HOOK_SOURCE_OUTPUT_UNLINK], o);
@@ -615,8 +626,7 @@ void pa_source_output_unlink(pa_source_output*o) {
if (o->client)
pa_idxset_remove_by_data(o->client->source_outputs, o, NULL);
- update_n_corked(o, PA_SOURCE_OUTPUT_UNLINKED);
- o->state = PA_SOURCE_OUTPUT_UNLINKED;
+ update_n_corked(o, old_state, PA_SOURCE_OUTPUT_UNLINKED);
if (linked && o->source) {
if (pa_source_output_is_passthrough(o))
@@ -700,7 +710,7 @@ void pa_source_output_put(pa_source_output *o) {
state = o->flags & PA_SOURCE_OUTPUT_START_CORKED ? PA_SOURCE_OUTPUT_CORKED : PA_SOURCE_OUTPUT_RUNNING;
- update_n_corked(o, state);
+ update_n_corked(o, o->state, state);
o->state = state;
/* We might need to update the source's volume if we are in flat volume mode. */
--
1.8.3.1
More information about the pulseaudio-discuss
mailing list