From e521d38787ff99ed1a8427eb9b6c72e958406e58 Mon Sep 17 00:00:00 2001 From: David Henningsson Date: Wed, 17 Sep 2014 08:56:51 +0200 Subject: srbchannel: Defer reading when setting up read callback Calling the callback while setting it up can make things complicated for clients, as the callback can do arbitrarily things. In this case, a protocol error caused the srbchannel to be owned by both the pstream and the native connection. Now the read callback is deferred, making sure the callback is called from a cleaner context where errors are handled appropriately. Reported-by: Tanu Kaskinen Signed-off-by: David Henningsson --- src/pulsecore/srbchannel.c | 24 +++++++++++++++++++++--- src/pulsecore/srbchannel.h | 3 --- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/pulsecore/srbchannel.c b/src/pulsecore/srbchannel.c index 3f81e256b..a0f916e76 100644 --- a/src/pulsecore/srbchannel.c +++ b/src/pulsecore/srbchannel.c @@ -86,6 +86,7 @@ struct pa_srbchannel { pa_srbchannel_cb_t callback; pa_io_event *read_event; + pa_defer_event *defer_event; pa_mainloop_api *mainloop; }; @@ -211,6 +212,17 @@ static void semread_cb(pa_mainloop_api *m, pa_io_event *e, int fd, pa_io_event_f srbchannel_rwloop(sr); } +static void defer_cb(pa_mainloop_api *m, pa_defer_event *e, void *userdata) { + pa_srbchannel* sr = userdata; + +#ifdef DEBUG_SRBCHANNEL + pa_log("Calling rw loop from deferred event"); +#endif + + m->defer_enable(e, 0); + srbchannel_rwloop(sr); +} + pa_srbchannel* pa_srbchannel_new(pa_mainloop_api *m, pa_mempool *p) { int capacity; int readfd; @@ -331,9 +343,13 @@ void pa_srbchannel_set_callback(pa_srbchannel *sr, pa_srbchannel_cb_t callback, sr->callback = callback; sr->cb_userdata = userdata; - if (sr->callback) - /* Maybe deferred event? */ - srbchannel_rwloop(sr); + if (sr->callback) { + /* If there are events to be read already in the ringbuffer, we will not get any IO event for that, + because that's how pa_fdsem works. Therefore check the ringbuffer in a defer event instead. */ + if (!sr->defer_event) + sr->defer_event = sr->mainloop->defer_new(sr->mainloop, defer_cb, sr); + sr->mainloop->defer_enable(sr->defer_event, 1); + } } void pa_srbchannel_free(pa_srbchannel *sr) @@ -343,6 +359,8 @@ void pa_srbchannel_free(pa_srbchannel *sr) #endif pa_assert(sr); + if (sr->defer_event) + sr->mainloop->defer_free(sr->defer_event); if (sr->read_event) sr->mainloop->io_free(sr->read_event); diff --git a/src/pulsecore/srbchannel.h b/src/pulsecore/srbchannel.h index e41cc5298..c96877e60 100644 --- a/src/pulsecore/srbchannel.h +++ b/src/pulsecore/srbchannel.h @@ -52,9 +52,6 @@ size_t pa_srbchannel_read(pa_srbchannel *sr, void *data, size_t l); * * Return false to abort all processing (e g if the srbchannel has been freed during the callback). * Otherwise return true. - * - * Note that the callback will be called immediately, to be able to process stuff that - * might already be in the buffer. */ typedef bool (*pa_srbchannel_cb_t)(pa_srbchannel *sr, void *userdata); void pa_srbchannel_set_callback(pa_srbchannel *sr, pa_srbchannel_cb_t callback, void *userdata); -- cgit v1.2.3