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