[4/8] tunnel*: put sink/source after authentication

Submitted by Yclept Nemo on July 17, 2018, 12:25 a.m.

Details

Message ID 20180717002544.5183-5-orbisvicis@gmail.com
State New
Series "*** Overview ***"
Headers show

Commit Message

Yclept Nemo July 17, 2018, 12:25 a.m.
Call 'pa_sink_put' or 'pa_source_put' after the connection is
authorized. For the new tunnel modules this involves sending a
module-specific message from the IO thread to the main thread. On
receipt the sink/source message handler calls 'pa_*_put' if connected
(this doesn't need to by guarded by a conditional; the message should
only be sent once). For the old tunnel module this involves moving
'pa_*_put' down the call chain of the main thread: 'pa__init' ->
'on_connection' -> 'setup_complete_callback'.
---
 src/modules/module-tunnel-sink-new.c   | 14 +++++++++++++-
 src/modules/module-tunnel-source-new.c | 14 +++++++++++++-
 src/modules/module-tunnel.c            | 12 ++++++------
 3 files changed, 32 insertions(+), 8 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/modules/module-tunnel-sink-new.c b/src/modules/module-tunnel-sink-new.c
index 802e6a59..b301f999 100644
--- a/src/modules/module-tunnel-sink-new.c
+++ b/src/modules/module-tunnel-sink-new.c
@@ -60,6 +60,10 @@  PA_MODULE_USAGE(
 #define MAX_LATENCY_USEC (200 * PA_USEC_PER_MSEC)
 #define TUNNEL_THREAD_FAILED_MAINLOOP 1
 
+enum {
+    SINK_MESSAGE_PUT = PA_SINK_MESSAGE_MAX,
+};
+
 static void stream_state_cb(pa_stream *stream, void *userdata);
 static void stream_changed_buffer_attr_cb(pa_stream *stream, void *userdata);
 static void stream_set_buffer_attr_cb(pa_stream *stream, int success, void *userdata);
@@ -345,6 +349,7 @@  static void context_state_cb(pa_context *c, void *userdata) {
                 u->thread_mainloop_api->quit(u->thread_mainloop_api, TUNNEL_THREAD_FAILED_MAINLOOP);
             }
             u->connected = true;
+            pa_asyncmsgq_send(u->thread_mq->outq, PA_MSGOBJECT(u->sink), SINK_MESSAGE_PUT, NULL, 0, NULL);
             break;
         }
         case PA_CONTEXT_FAILED:
@@ -402,6 +407,7 @@  static int sink_process_msg_cb(pa_msgobject *o, int code, void *data, int64_t of
     struct userdata *u = PA_SINK(o)->userdata;
 
     switch (code) {
+        /* Delivered from the main thread, handled in the IO thread. */
         case PA_SINK_MESSAGE_GET_LATENCY: {
             int negative;
             pa_usec_t remote_latency;
@@ -429,6 +435,13 @@  static int sink_process_msg_cb(pa_msgobject *o, int code, void *data, int64_t of
             *((int64_t*) data) = remote_latency;
             return 0;
         }
+
+        /* Delivered from the IO thread, handled in the main thread. */
+        case SINK_MESSAGE_PUT: {
+            if (u->connected)
+                pa_sink_put(u->sink);
+            return 0;
+        }
     }
     return pa_sink_process_msg(o, code, data, offset, chunk);
 }
@@ -572,7 +585,6 @@  int pa__init(pa_module *m) {
         goto fail;
     }
 
-    pa_sink_put(u->sink);
     pa_modargs_free(ma);
     pa_xfree(default_sink_name);
 
diff --git a/src/modules/module-tunnel-source-new.c b/src/modules/module-tunnel-source-new.c
index b41f53e2..4b2488d9 100644
--- a/src/modules/module-tunnel-source-new.c
+++ b/src/modules/module-tunnel-source-new.c
@@ -59,6 +59,10 @@  PA_MODULE_USAGE(
 
 #define TUNNEL_THREAD_FAILED_MAINLOOP 1
 
+enum {
+    SOURCE_MESSAGE_PUT = PA_SOURCE_MESSAGE_MAX,
+};
+
 static void stream_state_cb(pa_stream *stream, void *userdata);
 static void stream_read_cb(pa_stream *s, size_t length, void *userdata);
 static void context_state_cb(pa_context *c, void *userdata);
@@ -341,6 +345,7 @@  static void context_state_cb(pa_context *c, void *userdata) {
                 u->thread_mainloop_api->quit(u->thread_mainloop_api, TUNNEL_THREAD_FAILED_MAINLOOP);
             }
             u->connected = true;
+            pa_asyncmsgq_send(u->thread_mq->outq, PA_MSGOBJECT(u->source), SOURCE_MESSAGE_PUT, NULL, 0, NULL);
             break;
         }
         case PA_CONTEXT_FAILED:
@@ -397,6 +402,7 @@  static int source_process_msg_cb(pa_msgobject *o, int code, void *data, int64_t
     struct userdata *u = PA_SOURCE(o)->userdata;
 
     switch (code) {
+        /* Delivered from the main thread, handled in the IO thread. */
         case PA_SOURCE_MESSAGE_GET_LATENCY: {
             int negative;
             pa_usec_t remote_latency;
@@ -428,6 +434,13 @@  static int source_process_msg_cb(pa_msgobject *o, int code, void *data, int64_t
 
             return 0;
         }
+
+        /* Delivered from the IO thread, handled in the main thread. */
+        case SOURCE_MESSAGE_PUT: {
+            if (u->connected)
+                pa_source_put(u->source);
+            return 0;
+        }
     }
     return pa_source_process_msg(o, code, data, offset, chunk);
 }
@@ -566,7 +579,6 @@  int pa__init(pa_module *m) {
         goto fail;
     }
 
-    pa_source_put(u->source);
     pa_modargs_free(ma);
     pa_xfree(default_source_name);
 
diff --git a/src/modules/module-tunnel.c b/src/modules/module-tunnel.c
index 054d7d8f..960f8533 100644
--- a/src/modules/module-tunnel.c
+++ b/src/modules/module-tunnel.c
@@ -1598,6 +1598,12 @@  static void setup_complete_callback(pa_pdispatch *pd, uint32_t command, uint32_t
         goto fail;
     }
 
+#ifdef TUNNEL_SINK
+    pa_sink_put(u->sink);
+#else
+    pa_source_put(u->source);
+#endif
+
     /* Starting with protocol version 13 the MSB of the version tag
     reflects if shm is enabled for this connection or not. We don't
     support SHM here at all, so we just ignore this. */
@@ -2224,12 +2230,6 @@  int pa__init(pa_module*m) {
         goto fail;
     }
 
-#ifdef TUNNEL_SINK
-    pa_sink_put(u->sink);
-#else
-    pa_source_put(u->source);
-#endif
-
     pa_xfree(dn);
 
     if (server)

Comments

Tanu Kaskinen Aug. 12, 2018, 12:05 p.m.
On Mon, 2018-07-16 at 20:25 -0400, Yclept Nemo wrote:
> Call 'pa_sink_put' or 'pa_source_put' after the connection is
> authorized. For the new tunnel modules this involves sending a
> module-specific message from the IO thread to the main thread. On
> receipt the sink/source message handler calls 'pa_*_put' if connected
> (this doesn't need to by guarded by a conditional; the message should
> only be sent once). For the old tunnel module this involves moving
> 'pa_*_put' down the call chain of the main thread: 'pa__init' ->
> 'on_connection' -> 'setup_complete_callback'.
> ---
>  src/modules/module-tunnel-sink-new.c   | 14 +++++++++++++-
>  src/modules/module-tunnel-source-new.c | 14 +++++++++++++-
>  src/modules/module-tunnel.c            | 12 ++++++------
>  3 files changed, 32 insertions(+), 8 deletions(-)
> 
> diff --git a/src/modules/module-tunnel-sink-new.c b/src/modules/module-tunnel-sink-new.c
> index 802e6a59..b301f999 100644
> --- a/src/modules/module-tunnel-sink-new.c
> +++ b/src/modules/module-tunnel-sink-new.c
> @@ -60,6 +60,10 @@ PA_MODULE_USAGE(
>  #define MAX_LATENCY_USEC (200 * PA_USEC_PER_MSEC)
>  #define TUNNEL_THREAD_FAILED_MAINLOOP 1
>  
> +enum {
> +    SINK_MESSAGE_PUT = PA_SINK_MESSAGE_MAX,
> +};
> +
>  static void stream_state_cb(pa_stream *stream, void *userdata);
>  static void stream_changed_buffer_attr_cb(pa_stream *stream, void *userdata);
>  static void stream_set_buffer_attr_cb(pa_stream *stream, int success, void *userdata);
> @@ -345,6 +349,7 @@ static void context_state_cb(pa_context *c, void *userdata) {
>                  u->thread_mainloop_api->quit(u->thread_mainloop_api, TUNNEL_THREAD_FAILED_MAINLOOP);
>              }
>              u->connected = true;
> +            pa_asyncmsgq_send(u->thread_mq->outq, PA_MSGOBJECT(u->sink), SINK_MESSAGE_PUT, NULL, 0, NULL);

pa_asyncmsgq_send() is synchronous, and it shouldn't be used from the
IO thread, because that can cause deadlocks. I think an asynchronous
message would work fine in this case (i.e. pa_asyncmsgq_post()).

>              break;
>          }
>          case PA_CONTEXT_FAILED:
> @@ -402,6 +407,7 @@ static int sink_process_msg_cb(pa_msgobject *o, int code, void *data, int64_t of
>      struct userdata *u = PA_SINK(o)->userdata;
>  
>      switch (code) {
> +        /* Delivered from the main thread, handled in the IO thread. */
>          case PA_SINK_MESSAGE_GET_LATENCY: {
>              int negative;
>              pa_usec_t remote_latency;
> @@ -429,6 +435,13 @@ static int sink_process_msg_cb(pa_msgobject *o, int code, void *data, int64_t of
>              *((int64_t*) data) = remote_latency;
>              return 0;
>          }
> +
> +        /* Delivered from the IO thread, handled in the main thread. */
> +        case SINK_MESSAGE_PUT: {
> +            if (u->connected)
> +                pa_sink_put(u->sink);

u->connected is supposed to be used only in the IO thread. In what
situation would you want to not put the sink? I suppose at least during
module unloading it could happen that the SINK_MESSAGE_PUT message is
processed, but we don't want to call pa_sink_put().

A separate flag could be set in the beginning of pa__done() to indicate
that the module is being unloaded, but I think it would make sense to
have that flag in the core pa_module struct instead, and set it in
pa_module_free(). There's already pa_module.unload_requested, which is
kind of what we want, but it's currently only set if
pa_module_unload_request() is called. I think
pa_module.unload_requested could be set also from pa_module_free(), and
then it would be a reliable flag for checking whether the module is
being unloaded.