[pulseaudio-discuss] [PATCH] Don't allow unreferencing linked object.

[Juho Hämäläinen]

Sink(-input) and source(-output) called unlink function when reference
count dropped to zero. This would result in unlink hooks being called
with object that would have zero reference count. Some of the modules
for example already have asserts checking for proper refcount. As the
asserts from these modules are seemingly rare, better to just remove
the redundant unlinking code from sink(-input) and source(-output) and
assert on reference count in unlink functions as well.

Well behaving code if owner of said object should always unlink the object
before unreferencing.
---
 src/pulsecore/sink-input.c    | 6 ++----
 src/pulsecore/sink.c          | 6 ++----
 src/pulsecore/source-output.c | 7 +++----
 src/pulsecore/source.c        | 6 ++----
 4 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/src/pulsecore/sink-input.c b/src/pulsecore/sink-input.c
index 361b445..93bcc85 100644
--- a/src/pulsecore/sink-input.c
+++ b/src/pulsecore/sink-input.c
@@ -645,7 +645,7 @@ void pa_sink_input_unlink(pa_sink_input *i) {
     bool linked;
     pa_source_output *o, PA_UNUSED *p = NULL;
 
-    pa_assert(i);
+    pa_sink_input_assert_ref(i);
     pa_assert_ctl_context();
 
     /* See pa_sink_unlink() for a couple of comments how this function
@@ -721,9 +721,7 @@ static void sink_input_free(pa_object *o) {
     pa_assert(i);
     pa_assert_ctl_context();
     pa_assert(pa_sink_input_refcnt(i) == 0);
-
-    if (PA_SINK_INPUT_IS_LINKED(i->state))
-        pa_sink_input_unlink(i);
+    pa_assert(!PA_SINK_INPUT_IS_LINKED(i->state));
 
     pa_log_info("Freeing input %u \"%s\"", i->index,
                 i->proplist ? pa_strnull(pa_proplist_gets(i->proplist, PA_PROP_MEDIA_NAME)) : "");
diff --git a/src/pulsecore/sink.c b/src/pulsecore/sink.c
index 3f1ef72..9bdf9be 100644
--- a/src/pulsecore/sink.c
+++ b/src/pulsecore/sink.c
@@ -669,7 +669,7 @@ void pa_sink_unlink(pa_sink* s) {
     bool linked;
     pa_sink_input *i, PA_UNUSED *j = NULL;
 
-    pa_assert(s);
+    pa_sink_assert_ref(s);
     pa_assert_ctl_context();
 
     /* Please note that pa_sink_unlink() does more than simply
@@ -722,9 +722,7 @@ static void sink_free(pa_object *o) {
     pa_assert(s);
     pa_assert_ctl_context();
     pa_assert(pa_sink_refcnt(s) == 0);
-
-    if (PA_SINK_IS_LINKED(s->state))
-        pa_sink_unlink(s);
+    pa_assert(!PA_SINK_IS_LINKED(s->state));
 
     pa_log_info("Freeing sink %u \"%s\"", s->index, s->name);
 
diff --git a/src/pulsecore/source-output.c b/src/pulsecore/source-output.c
index d74a60e..1478469 100644
--- a/src/pulsecore/source-output.c
+++ b/src/pulsecore/source-output.c
@@ -548,7 +548,8 @@ 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) {
     bool linked;
-    pa_assert(o);
+
+    pa_source_output_assert_ref(o);
     pa_assert_ctl_context();
 
     /* See pa_sink_unlink() for a couple of comments how this function
@@ -614,9 +615,7 @@ static void source_output_free(pa_object* mo) {
     pa_assert(o);
     pa_assert_ctl_context();
     pa_assert(pa_source_output_refcnt(o) == 0);
-
-    if (PA_SOURCE_OUTPUT_IS_LINKED(o->state))
-        pa_source_output_unlink(o);
+    pa_assert(!PA_SOURCE_OUTPUT_IS_LINKED(o->state));
 
     pa_log_info("Freeing output %u \"%s\"", o->index,
                 o->proplist ? pa_strnull(pa_proplist_gets(o->proplist, PA_PROP_MEDIA_NAME)) : "");
diff --git a/src/pulsecore/source.c b/src/pulsecore/source.c
index 98374ae..8a527d8 100644
--- a/src/pulsecore/source.c
+++ b/src/pulsecore/source.c
@@ -612,7 +612,7 @@ void pa_source_unlink(pa_source *s) {
     bool linked;
     pa_source_output *o, PA_UNUSED *j = NULL;
 
-    pa_assert(s);
+    pa_source_assert_ref(s);
     pa_assert_ctl_context();
 
     /* See pa_sink_unlink() for a couple of comments how this function
@@ -661,9 +661,7 @@ static void source_free(pa_object *o) {
     pa_assert(s);
     pa_assert_ctl_context();
     pa_assert(pa_source_refcnt(s) == 0);
-
-    if (PA_SOURCE_IS_LINKED(s->state))
-        pa_source_unlink(s);
+    pa_assert(!PA_SOURCE_IS_LINKED(s->state));
 
     pa_log_info("Freeing source %u \"%s\"", s->index, s->name);
 
-- 
2.1.4