[pulseaudio-discuss] [PATCH] pstream: fix revoke callback setting

[Tanu Kaskinen]

While investigating bug 95352, I noticed that
pa_pstream_set_revoke_callback() and pa_pstream_set_release_callback()
were identical - both set the release callback.
pa_pstream_set_revoke_callback() was obviously broken - it was setting
the wrong callback.

The only place where set_revoke_callback() is called is in
protocol-native.c. The code there looks like this:

    pa_pstream_set_revoke_callback(c->pstream, pstream_revoke_callback, c);
    pa_pstream_set_release_callback(c->pstream, pstream_release_callback, c);

Since set_release_callback() is called last, the release callback gets
set correctly. The only problem is that the revoke callback stays
unset. What are the consequences of that? The code that calls the
revoke callback looks like this:

    if (p->revoke_callback)
        p->revoke_callback(p, block_id, p->revoke_callback_userdata);
    else
        pa_pstream_send_revoke(p, block_id);

So the intended callback is replaced with a pa_pstream_send_revoke()
call. What does the intended callback, that doesn't get called, do?

    if (!(q = pa_thread_mq_get()))
        pa_pstream_send_revoke(p, block_id);
    else
        pa_asyncmsgq_post(q->outq, PA_MSGOBJECT(userdata), CONNECTION_MESSAGE_REVOKE, PA_UINT_TO_PTR(block_id), 0, NULL, NULL);

So the native protocol's revoke callback is anyway going to call
pa_pstream_send_revoke() when called from the main thread. If the
revoking is done from an IO thread, an asynchronous message is sent to
the main thread instead, and the message handler will then call
pa_pstream_send_revoke().

In conclusion, the only effect of this bug was that
pa_pstream_send_revoke() was sometimes being called from an IO thread
when it should have been called from the main thread. I don't know if
this caused the crash in bug 95352. Probably not.
---
 src/pulsecore/pstream.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/pulsecore/pstream.c b/src/pulsecore/pstream.c
index 98a8382..d5200d7 100644
--- a/src/pulsecore/pstream.c
+++ b/src/pulsecore/pstream.c
@@ -994,8 +994,8 @@ void pa_pstream_set_revoke_callback(pa_pstream *p, pa_pstream_block_id_cb_t cb,
     pa_assert(p);
     pa_assert(PA_REFCNT_VALUE(p) > 0);
 
-    p->release_callback = cb;
-    p->release_callback_userdata = userdata;
+    p->revoke_callback = cb;
+    p->revoke_callback_userdata = userdata;
 }
 
 bool pa_pstream_is_pending(pa_pstream *p) {
-- 
2.8.1