[pulseaudio-commits] [Git][pulseaudio/pulseaudio][master] 3 commits: tunnel modules: Fix threading issues

PulseAudio Marge Bot (@pulseaudio-merge-bot) gitlab at gitlab.freedesktop.org
Wed May 25 07:12:17 UTC 2022



PulseAudio Marge Bot pushed to branch master at PulseAudio / pulseaudio


Commits:
c3d1db2f by Georg Chini at 2022-05-25T07:04:09+00:00
tunnel modules: Fix threading issues

The old tunnel modules switched wrongly between main thread and I/O-thread
while the new tunnel modules sent unnecessary messages to the main thread.
This patch fixes the issues.

Part-of: <https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/merge_requests/705>

- - - - -
363a3841 by Georg Chini at 2022-05-25T07:04:09+00:00
tunnel modules: Fix crash when the module was unloaded while waiting for re-init

When the tunnel modules had no connection and a re-init was pending, the module
could be unloaded without cancelling the pending re-init. When the timer expired
in that situation, this lead to a crash. This patch fixes the problem by keeping
a reference when the module is scheduled to be re-initialized.

Part-of: <https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/merge_requests/705>

- - - - -
5bba8ee6 by Georg Chini at 2022-05-25T07:04:09+00:00
module-tunnel: Improve latency calculation

The timestamp used for updating the smoother was taken at the wrong time.
It may take some time until an async message is executed (measured up to
2ms), therefore the timestamp used to update the smoother must be taken
before the message is executed and not inside the message.

Part-of: <https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/merge_requests/705>

- - - - -


5 changed files:

- src/modules/module-tunnel-sink-new.c
- src/modules/module-tunnel-source-new.c
- src/modules/module-tunnel.c
- src/modules/restart-module.c
- src/modules/restart-module.h


Changes:

=====================================
src/modules/module-tunnel-sink-new.c
=====================================
@@ -118,6 +118,11 @@ struct userdata {
     pa_usec_t reconnect_interval_us;
 };
 
+struct module_restart_data {
+    struct userdata *userdata;
+    pa_restart_data *restart_data;
+};
+
 static const char* const valid_modargs[] = {
     "sink_name",
     "sink_properties",
@@ -337,12 +342,21 @@ static void stream_overflow_callback(pa_stream *stream, void *userdata) {
 
 /* Do a reinit of the module.  Note that u will be freed as a result of this
  * call. */
-static void maybe_restart(struct userdata *u) {
+static void maybe_restart(struct module_restart_data *rd) {
+    struct userdata *u = rd->userdata;
+
+    if (rd->restart_data) {
+        pa_log_debug("Restart already pending");
+        return;
+    }
+
     if (u->reconnect_interval_us > 0) {
-        pa_restart_module_reinit(u->module, do_init, do_done, u->reconnect_interval_us);
+        /* The handle returned here must be freed when do_init() finishes successfully
+         * and when the module exits. */
+        rd->restart_data = pa_restart_module_reinit(u->module, do_init, do_done, u->reconnect_interval_us);
     } else {
         /* exit the module */
-        pa_asyncmsgq_post(u->thread_mq->outq, PA_MSGOBJECT(u->module->core), PA_CORE_MESSAGE_UNLOAD_MODULE, u->module, 0, NULL, NULL);
+        pa_module_unload_request(u->module, true);
     }
 }
 
@@ -620,7 +634,7 @@ static int tunnel_process_msg(pa_msgobject *o, int code, void *data, int64_t off
             create_sink(u);
             break;
         case TUNNEL_MESSAGE_MAYBE_RESTART:
-            maybe_restart(u);
+            maybe_restart(u->module->userdata);
             break;
     }
 
@@ -629,12 +643,16 @@ static int tunnel_process_msg(pa_msgobject *o, int code, void *data, int64_t off
 
 static int do_init(pa_module *m) {
     struct userdata *u = NULL;
+    struct module_restart_data *rd;
     pa_modargs *ma = NULL;
     const char *remote_server = NULL;
     char *default_sink_name = NULL;
     uint32_t reconnect_interval_ms = 0;
 
     pa_assert(m);
+    pa_assert(m->userdata);
+
+    rd = m->userdata;
 
     if (!(ma = pa_modargs_new(m->argument, valid_modargs))) {
         pa_log("Failed to parse module arguments.");
@@ -643,7 +661,7 @@ static int do_init(pa_module *m) {
 
     u = pa_xnew0(struct userdata, 1);
     u->module = m;
-    m->userdata = u;
+    rd->userdata = u;
 
     u->sample_spec = m->core->default_sample_spec;
     u->channel_map = m->core->default_channel_map;
@@ -711,6 +729,16 @@ static int do_init(pa_module *m) {
         goto fail;
     }
 
+    /* If the module is restarting and do_init() finishes successfully, the
+     * restart data is no longer needed. If do_init() fails, don't touch the
+     * restart data, because following restart attempts will continue to use
+     * the same data. If restart_data is NULL, that means no restart is
+     * currently pending. */
+    if (rd->restart_data) {
+        pa_restart_free(rd->restart_data);
+        rd->restart_data = NULL;
+    }
+
     pa_modargs_free(ma);
     pa_xfree(default_sink_name);
 
@@ -728,10 +756,13 @@ fail:
 
 static void do_done(pa_module *m) {
     struct userdata *u = NULL;
+    struct module_restart_data *rd;
 
     pa_assert(m);
 
-    if (!(u = m->userdata))
+    if (!(rd = m->userdata))
+        return;
+    if (!(u = rd->userdata))
         return;
 
     u->shutting_down = true;
@@ -777,7 +808,7 @@ static void do_done(pa_module *m) {
 
     pa_xfree(u);
 
-    m->userdata = NULL;
+    rd->userdata = NULL;
 }
 
 int pa__init(pa_module *m) {
@@ -785,6 +816,8 @@ int pa__init(pa_module *m) {
 
     pa_assert(m);
 
+    m->userdata = pa_xnew0(struct module_restart_data, 1);
+
     ret = do_init(m);
 
     if (ret < 0)
@@ -797,4 +830,13 @@ void pa__done(pa_module *m) {
     pa_assert(m);
 
     do_done(m);
+
+    if (m->userdata) {
+        struct module_restart_data *rd = m->userdata;
+
+        if (rd->restart_data)
+            pa_restart_free(rd->restart_data);
+
+        pa_xfree(m->userdata);
+    }
 }


=====================================
src/modules/module-tunnel-source-new.c
=====================================
@@ -116,6 +116,11 @@ struct userdata {
     pa_usec_t reconnect_interval_us;
 };
 
+struct module_restart_data {
+    struct userdata *userdata;
+    pa_restart_data *restart_data;
+};
+
 static const char* const valid_modargs[] = {
     "source_name",
     "source_properties",
@@ -320,15 +325,22 @@ static void stream_state_cb(pa_stream *stream, void *userdata) {
 }
 
 /* Do a reinit of the module.  Note that u will be freed as a result of this
- * call, while pu will live on to the next iteration.  It's up to do_done to
- * copy anything that we want to persist across iterations out of u and into pu
- */
-static void maybe_restart(struct userdata *u) {
+ * call. */
+static void maybe_restart(struct module_restart_data *rd) {
+    struct userdata *u = rd->userdata;
+
+    if (rd->restart_data) {
+        pa_log_debug("Restart already pending");
+        return;
+    }
+
     if (u->reconnect_interval_us > 0) {
-        pa_restart_module_reinit(u->module, do_init, do_done, u->reconnect_interval_us);
+        /* The handle returned here must be freed when do_init() finishes successfully
+         * and when the module exits. */
+        rd->restart_data = pa_restart_module_reinit(u->module, do_init, do_done, u->reconnect_interval_us);
     } else {
         /* exit the module */
-        pa_asyncmsgq_post(u->thread_mq->outq, PA_MSGOBJECT(u->module->core), PA_CORE_MESSAGE_UNLOAD_MODULE, u->module, 0, NULL, NULL);
+        pa_module_unload_request(u->module, true);
     }
 }
 
@@ -594,7 +606,7 @@ static int tunnel_process_msg(pa_msgobject *o, int code, void *data, int64_t off
             create_source(u);
             break;
         case TUNNEL_MESSAGE_MAYBE_RESTART:
-            maybe_restart(u);
+            maybe_restart(u->module->userdata);
             break;
     }
 
@@ -603,12 +615,16 @@ static int tunnel_process_msg(pa_msgobject *o, int code, void *data, int64_t off
 
 static int do_init(pa_module *m) {
     struct userdata *u = NULL;
+    struct module_restart_data *rd;
     pa_modargs *ma = NULL;
     const char *remote_server = NULL;
     char *default_source_name = NULL;
     uint32_t reconnect_interval_ms = 0;
 
     pa_assert(m);
+    pa_assert(m->userdata);
+
+    rd = m->userdata;
 
     if (!(ma = pa_modargs_new(m->argument, valid_modargs))) {
         pa_log("Failed to parse module arguments.");
@@ -617,7 +633,7 @@ static int do_init(pa_module *m) {
 
     u = pa_xnew0(struct userdata, 1);
     u->module = m;
-    m->userdata = u;
+    rd->userdata = u;
 
     u->sample_spec = m->core->default_sample_spec;
     u->channel_map = m->core->default_channel_map;
@@ -682,6 +698,16 @@ static int do_init(pa_module *m) {
         goto fail;
     }
 
+    /* If the module is restarting and do_init() finishes successfully, the
+     * restart data is no longer needed. If do_init() fails, don't touch the
+     * restart data, because following restart attempts will continue to use
+     * the same data. If restart_data is NULL, that means no restart is
+     * currently pending. */
+    if (rd->restart_data) {
+        pa_restart_free(rd->restart_data);
+        rd->restart_data = NULL;
+    }
+
     pa_modargs_free(ma);
     pa_xfree(default_source_name);
 
@@ -699,10 +725,13 @@ fail:
 
 static void do_done(pa_module *m) {
     struct userdata *u = NULL;
+    struct module_restart_data *rd;
 
     pa_assert(m);
 
-    if (!(u = m->userdata))
+    if (!(rd = m->userdata))
+        return;
+    if (!(u = rd->userdata))
         return;
 
     u->shutting_down = true;
@@ -748,7 +777,7 @@ static void do_done(pa_module *m) {
 
     pa_xfree(u);
 
-    m->userdata = NULL;
+    rd->userdata = NULL;
 }
 
 int pa__init(pa_module *m) {
@@ -756,6 +785,8 @@ int pa__init(pa_module *m) {
 
     pa_assert(m);
 
+    m->userdata = pa_xnew0(struct module_restart_data, 1);
+
     ret = do_init(m);
 
     if (ret < 0)
@@ -768,4 +799,13 @@ void pa__done(pa_module *m) {
     pa_assert(m);
 
     do_done(m);
+
+    if (m->userdata) {
+        struct module_restart_data *rd = m->userdata;
+
+        if (rd->restart_data)
+            pa_restart_free(rd->restart_data);
+
+        pa_xfree(m->userdata);
+    }
 }


=====================================
src/modules/module-tunnel.c
=====================================
@@ -147,7 +147,6 @@ enum {
     SINK_MESSAGE_UPDATE_LATENCY,
     SINK_MESSAGE_GET_LATENCY_SNAPSHOT,
     SINK_MESSAGE_POST,
-    SINK_MESSAGE_CREATED
 };
 
 #define DEFAULT_LATENCY_MSEC 100
@@ -159,7 +158,6 @@ enum {
     SOURCE_MESSAGE_REMOTE_SUSPEND,
     SOURCE_MESSAGE_UPDATE_LATENCY,
     SOURCE_MESSAGE_GET_LATENCY_SNAPSHOT,
-    SOURCE_MESSAGE_CREATED
 };
 
 #define DEFAULT_LATENCY_MSEC 25
@@ -174,11 +172,6 @@ typedef struct tunnel_msg tunnel_msg;
 PA_DEFINE_PRIVATE_CLASS(tunnel_msg, pa_msgobject);
 
 enum {
-#ifdef TUNNEL_SINK
-    TUNNEL_MESSAGE_CREATE_SINK_REQUEST,
-#else
-    TUNNEL_MESSAGE_CREATE_SOURCE_REQUEST,
-#endif
     TUNNEL_MESSAGE_MAYBE_RESTART,
 };
 
@@ -297,25 +290,39 @@ struct userdata {
     pa_iochannel *io;
 
     pa_usec_t reconnect_interval_us;
+    pa_usec_t snapshot_time;
+};
+
+struct module_restart_data {
+    struct userdata *userdata;
+    pa_restart_data *restart_data;
 };
 
 static void request_latency(struct userdata *u);
 #ifdef TUNNEL_SINK
+static void create_sink(struct userdata *u);
 static void on_sink_created(struct userdata *u);
 #else
+static void create_source(struct userdata *u);
 static void on_source_created(struct userdata *u);
 #endif
 
 /* Do a reinit of the module.  Note that u will be freed as a result of this
- * call, while pu will live on to the next iteration.  It's up to do_done to
- * copy anything that we want to persist across iterations out of u and into pu
- */
-static void unload_module(struct userdata *u) {
+ * call. */
+static void unload_module(struct module_restart_data *rd) {
+    struct userdata *u = rd->userdata;
+
+    if (rd->restart_data) {
+        pa_log_debug("Restart already pending");
+        return;
+    }
+
     if (u->reconnect_interval_us > 0) {
-        pa_restart_module_reinit(u->module, do_init, do_done, u->reconnect_interval_us);
-    } else {
+        /* The handle returned here must be freed when do_init() was successful and when the
+         * module exits. */
+        rd->restart_data = pa_restart_module_reinit(u->module, do_init, do_done, u->reconnect_interval_us);
+    } else
         pa_module_unload_request(u->module, true);
-    }
 }
 
 /* Called from main context */
@@ -333,7 +340,7 @@ static void command_stream_killed(pa_pdispatch *pd,  uint32_t command,  uint32_t
     pa_assert(u->pdispatch == pd);
 
     pa_log_warn("Stream killed");
-    unload_module(u);
+    unload_module(u->module->userdata);
 }
 
 /* Called from main context */
@@ -365,7 +372,7 @@ static void command_suspended(pa_pdispatch *pd,  uint32_t command,  uint32_t tag
         !pa_tagstruct_eof(t)) {
 
         pa_log("Invalid packet.");
-        unload_module(u);
+        unload_module(u->module->userdata);
         return;
     }
 
@@ -398,7 +405,7 @@ static void command_moved(pa_pdispatch *pd,  uint32_t command,  uint32_t tag, pa
         pa_tagstruct_get_boolean(t, &suspended) < 0) {
 
         pa_log_error("Invalid packet.");
-        unload_module(u);
+        unload_module(u->module->userdata);
         return;
     }
 
@@ -427,7 +434,7 @@ static void command_stream_buffer_attr_changed(pa_pdispatch *pd, uint32_t comman
         pa_tagstruct_getu32(t, &maxlength) < 0) {
 
         pa_log_error("Invalid packet.");
-        unload_module(u);
+        unload_module(u->module->userdata);
         return;
     }
 
@@ -436,7 +443,7 @@ static void command_stream_buffer_attr_changed(pa_pdispatch *pd, uint32_t comman
             pa_tagstruct_get_usec(t, &usec) < 0) {
 
             pa_log_error("Invalid packet.");
-            unload_module(u);
+            unload_module(u->module->userdata);
             return;
         }
     } else {
@@ -446,7 +453,7 @@ static void command_stream_buffer_attr_changed(pa_pdispatch *pd, uint32_t comman
             pa_tagstruct_get_usec(t, &usec) < 0) {
 
             pa_log_error("Invalid packet.");
-            unload_module(u);
+            unload_module(u->module->userdata);
             return;
         }
     }
@@ -640,7 +647,8 @@ static int sink_process_msg(pa_msgobject *o, int code, void *data, int64_t offse
             else
                 bytes = 0;
 
-             pa_smoother_2_put(u->smoother, pa_rtclock_now(), bytes);
+            /* We may use u->snapshot time because the main thread is waiting */
+             pa_smoother_2_put(u->smoother, u->snapshot_time, bytes);
 #else
             pa_usec_t y;
 
@@ -651,7 +659,8 @@ static int sink_process_msg(pa_msgobject *o, int code, void *data, int64_t offse
             else
                 y = 0;
 
-            pa_smoother_put(u->smoother, pa_rtclock_now(), y);
+            /* We may use u->snapshot time because the main thread is waiting */
+            pa_smoother_put(u->smoother, u->snapshot_time, y);
 #endif
 
             /* We can access this freely here, since the main thread is waiting for us */
@@ -671,12 +680,6 @@ static int sink_process_msg(pa_msgobject *o, int code, void *data, int64_t offse
 
             u->receive_counter += chunk->length;
 
-            return 0;
-
-        case SINK_MESSAGE_CREATED:
-
-            on_sink_created(u);
-
             return 0;
     }
 
@@ -790,14 +793,16 @@ static int source_process_msg(pa_msgobject *o, int code, void *data, int64_t off
 
             bytes += u->counter;
 
-            pa_smoother_2_put(u->smoother, pa_rtclock_now(), bytes);
+            /* We may use u->snapshot time because the main thread is waiting */
+            pa_smoother_2_put(u->smoother, u->snapshot_time, bytes);
 #else
             pa_usec_t y;
 
             y = pa_bytes_to_usec((uint64_t) u->counter, &u->source->sample_spec);
             y += offset;
 
-            pa_smoother_put(u->smoother, pa_rtclock_now(), y);
+            /* We may use u->snapshot time because the main thread is waiting */
+            pa_smoother_put(u->smoother, u->snapshot_time, y);
 #endif
 
             /* We can access this freely here, since the main thread is waiting for us */
@@ -805,12 +810,6 @@ static int source_process_msg(pa_msgobject *o, int code, void *data, int64_t off
 
             return 0;
         }
-
-        case SOURCE_MESSAGE_CREATED:
-
-            on_source_created(u);
-
-            return 0;
     }
 
     return pa_source_process_msg(o, code, data, offset, chunk);
@@ -912,7 +911,7 @@ static void command_request(pa_pdispatch *pd, uint32_t command,  uint32_t tag, p
     return;
 
 fail:
-    unload_module(u);
+    unload_module(u->module->userdata);
 }
 
 #endif
@@ -1017,6 +1016,9 @@ static void stream_get_latency_callback(pa_pdispatch *pd, uint32_t command, uint
     delay += (int64_t) pa_bytes_to_usec(send_counter - u->receive_snapshot, ss);
 #endif
 
+    /* It may take some time before the async message is executed, so we take a timestamp here */
+    u->snapshot_time = pa_rtclock_now();
+
 #ifdef TUNNEL_SINK
     pa_asyncmsgq_send(u->sink->asyncmsgq, PA_MSGOBJECT(u->sink), SINK_MESSAGE_UPDATE_LATENCY, 0, delay, NULL);
 #else
@@ -1027,7 +1029,7 @@ static void stream_get_latency_callback(pa_pdispatch *pd, uint32_t command, uint
 
 fail:
 
-    unload_module(u);
+    unload_module(u->module->userdata);
 }
 
 /* Called from main context */
@@ -1162,7 +1164,7 @@ static void server_info_cb(pa_pdispatch *pd, uint32_t command,  uint32_t tag, pa
     return;
 
 fail:
-    unload_module(u);
+    unload_module(u->module->userdata);
 }
 
 static int read_ports(struct userdata *u, pa_tagstruct *t) {
@@ -1317,7 +1319,7 @@ static void sink_info_cb(pa_pdispatch *pd, uint32_t command,  uint32_t tag, pa_t
     return;
 
 fail:
-    unload_module(u);
+    unload_module(u->module->userdata);
 }
 
 /* Called from main context */
@@ -1426,7 +1428,7 @@ static void sink_input_info_cb(pa_pdispatch *pd, uint32_t command,  uint32_t tag
     return;
 
 fail:
-    unload_module(u);
+    unload_module(u->module->userdata);
 }
 
 #else
@@ -1516,7 +1518,7 @@ static void source_info_cb(pa_pdispatch *pd, uint32_t command,  uint32_t tag, pa
     return;
 
 fail:
-    unload_module(u);
+    unload_module(u->module->userdata);
 }
 
 #endif
@@ -1577,7 +1579,7 @@ static void command_subscribe_event(pa_pdispatch *pd,  uint32_t command,  uint32
     if (pa_tagstruct_getu32(t, &e) < 0 ||
         pa_tagstruct_getu32(t, &idx) < 0) {
         pa_log("Invalid protocol reply");
-        unload_module(u);
+        unload_module(u->module->userdata);
         return;
     }
 
@@ -1724,7 +1726,7 @@ parse_error:
     pa_log("Invalid reply. (Create stream)");
 
 fail:
-    unload_module(u);
+    unload_module(u->module->userdata);
 
 }
 
@@ -1928,7 +1930,7 @@ static void setup_complete_callback(pa_pdispatch *pd, uint32_t command, uint32_t
     return;
 
 fail:
-    unload_module(u);
+    unload_module(u->module->userdata);
 }
 
 /* Called from main context */
@@ -1939,7 +1941,7 @@ static void pstream_die_callback(pa_pstream *p, void *userdata) {
     pa_assert(u);
 
     pa_log_warn("Stream died.");
-    unload_module(u);
+    unload_module(u->module->userdata);
 }
 
 /* Called from main context */
@@ -1952,7 +1954,7 @@ static void pstream_packet_callback(pa_pstream *p, pa_packet *packet, pa_cmsg_an
 
     if (pa_pdispatch_run(u->pdispatch, packet, ancil_data, u) < 0) {
         pa_log("Invalid packet");
-        unload_module(u);
+        unload_module(u->module->userdata);
         return;
     }
 }
@@ -1968,7 +1970,7 @@ static void pstream_memblock_callback(pa_pstream *p, uint32_t channel, int64_t o
 
     if (channel != u->channel) {
         pa_log("Received memory block on bad channel.");
-        unload_module(u);
+        unload_module(u->module->userdata);
         return;
     }
 
@@ -1982,6 +1984,8 @@ static void pstream_memblock_callback(pa_pstream *p, uint32_t channel, int64_t o
 static void on_connection(pa_socket_client *sc, pa_iochannel *io, void *userdata) {
     struct userdata *u = userdata;
 
+    pa_assert_ctl_context();
+
     pa_assert(sc);
     pa_assert(u);
     pa_assert(u->client == sc);
@@ -1991,18 +1995,26 @@ static void on_connection(pa_socket_client *sc, pa_iochannel *io, void *userdata
 
     if (!io) {
         pa_log("Connection failed: %s", pa_cstrerror(errno));
-        unload_module(u);
+        unload_module(u->module->userdata);
         return;
     }
 
     u->io = io;
 
 #ifdef TUNNEL_SINK
-    pa_log_debug("Asking ctl thread to create sink.");
-    pa_asyncmsgq_post(u->thread_mq.outq, PA_MSGOBJECT(u->msg), TUNNEL_MESSAGE_CREATE_SINK_REQUEST, u, 0, NULL, NULL);
+    create_sink(u);
+    if (!u->sink) {
+        unload_module(u->module->userdata);
+        return;
+    }
+    on_sink_created(u);
 #else
-    pa_log_debug("Asking ctl thread to create source.");
-    pa_asyncmsgq_post(u->thread_mq.outq, PA_MSGOBJECT(u->msg), TUNNEL_MESSAGE_CREATE_SOURCE_REQUEST, u, 0, NULL, NULL);
+    create_source(u);
+    if (!u->source) {
+        unload_module(u->module->userdata);
+        return;
+    }
+    on_source_created(u);
 #endif
 }
 
@@ -2141,8 +2153,6 @@ static void create_sink(struct userdata *u) {
 finish:
     pa_sink_new_data_done(&data);
     pa_xfree(data_name);
-
-    pa_asyncmsgq_post(u->sink->asyncmsgq, PA_MSGOBJECT(u->sink), SINK_MESSAGE_CREATED, u, 0, NULL, NULL);
 }
 #else
 static void create_source(struct userdata *u) {
@@ -2190,8 +2200,6 @@ static void create_source(struct userdata *u) {
 finish:
     pa_source_new_data_done(&data);
     pa_xfree(data_name);
-
-    pa_asyncmsgq_post(u->source->asyncmsgq, PA_MSGOBJECT(u->source), SOURCE_MESSAGE_CREATED, u, 0, NULL, NULL);
 }
 #endif
 
@@ -2206,17 +2214,9 @@ static int tunnel_process_msg(pa_msgobject *o, int code, void *data, int64_t off
         return 0;
 
     switch (code) {
-#ifdef TUNNEL_SINK
-        case TUNNEL_MESSAGE_CREATE_SINK_REQUEST:
-            create_sink(u);
-            break;
-#else
-        case TUNNEL_MESSAGE_CREATE_SOURCE_REQUEST:
-            create_source(u);
-            break;
-#endif
+
         case TUNNEL_MESSAGE_MAYBE_RESTART:
-            unload_module(u);
+            unload_module(u->module->userdata);
             break;
     }
 
@@ -2258,7 +2258,10 @@ static int start_connect(struct userdata *u, char *server, bool automatic) {
         server_list = pa_strlist_pop(server_list, &u->server_name);
 
         if (!u->server_name) {
-            pa_log("Failed to connect to server '%s'", server);
+            if (server)
+                pa_log("Failed to connect to server '%s'", server);
+            else
+                pa_log("Failed to connect");
             rc = -1;
             goto done;
         }
@@ -2286,6 +2289,7 @@ done:
 static int do_init(pa_module *m) {
     pa_modargs *ma = NULL;
     struct userdata *u = NULL;
+    struct module_restart_data *rd;
     char *server = NULL;
     uint32_t latency_msec;
     bool automatic;
@@ -2296,13 +2300,16 @@ static int do_init(pa_module *m) {
     uint32_t reconnect_interval_ms = 0;
 
     pa_assert(m);
+    pa_assert(m->userdata);
+
+    rd = m->userdata;
 
     if (!(ma = pa_modargs_new(m->argument, valid_modargs))) {
         pa_log("Failed to parse module arguments");
         goto fail;
     }
 
-    m->userdata = u = pa_xnew0(struct userdata, 1);
+    rd->userdata = u = pa_xnew0(struct userdata, 1);
     u->core = m->core;
     u->module = m;
     u->client = NULL;
@@ -2508,6 +2515,16 @@ static int do_init(pa_module *m) {
         xcb_disconnect(xcb);
 #endif
 
+    /* If the module is restarting and do_init() finishes successfully, the
+     * restart data is no longer needed. If do_init() fails, don't touch the
+     * restart data, because following restart attempts will continue to use
+     * the same data. If restart_data is NULL, that means no restart is
+     * currently pending. */
+    if (rd->restart_data) {
+        pa_restart_free(rd->restart_data);
+        rd->restart_data = NULL;
+    }
+
     pa_modargs_free(ma);
 
     return 0;
@@ -2529,10 +2546,13 @@ fail:
 
 static void do_done(pa_module *m) {
     struct userdata *u = NULL;
+    struct module_restart_data *rd;
 
     pa_assert(m);
 
-    if (!(u = m->userdata))
+    if (!(rd = m->userdata))
+        return;
+    if (!(u = rd->userdata))
         return;
 
     u->shutting_down = true;
@@ -2611,7 +2631,7 @@ static void do_done(pa_module *m) {
 
     pa_xfree(u);
 
-    m->userdata = NULL;
+    rd->userdata = NULL;
 }
 
 int pa__init(pa_module *m) {
@@ -2619,6 +2639,8 @@ int pa__init(pa_module *m) {
 
     pa_assert(m);
 
+    m->userdata = pa_xnew0(struct module_restart_data, 1);
+
     ret = do_init(m);
 
     if (ret < 0)
@@ -2631,4 +2653,13 @@ void pa__done(pa_module *m) {
     pa_assert(m);
 
     do_done(m);
+
+    if (m->userdata) {
+        struct module_restart_data *rd = m->userdata;
+
+        if (rd->restart_data)
+            pa_restart_free(rd->restart_data);
+
+        pa_xfree(m->userdata);
+    }
 }


=====================================
src/modules/restart-module.c
=====================================
@@ -30,31 +30,49 @@
 #include <pulsecore/core.h>
 #include <pulsecore/thread-mq.h>
 
-struct reinit_data {
+struct pa_restart_data {
     init_cb do_init;
     done_cb do_done;
 
     pa_usec_t restart_usec;
     pa_module *module;
+    pa_time_event *time_event;
+    pa_defer_event *defer_event;
 };
 
+static void do_reinit(pa_mainloop_api *mainloop, pa_restart_data *rd);
+
 static void call_init(pa_mainloop_api *mainloop, pa_time_event *e, const struct timeval *tv, void *userdata) {
-    struct reinit_data *rd = userdata;
+    pa_restart_data *rd = userdata;
     int ret;
 
+    if (rd->time_event) {
+        mainloop->time_free(rd->time_event);
+        rd->time_event = NULL;
+    }
+
     /* now that restart_usec has elapsed, we call do_init to restart the module */
     ret = rd->do_init(rd->module);
 
     /* if the init failed, we got here because the caller wanted to restart, so
      * setup another restart */
     if (ret < 0)
-        pa_restart_module_reinit(rd->module, rd->do_init, rd->do_done, rd->restart_usec);
+        do_reinit(mainloop, rd);
+}
 
-    pa_xfree(rd);
+static void defer_callback(pa_mainloop_api *mainloop, pa_defer_event *e, void *userdata) {
+    pa_restart_data *rd = userdata;
+
+    pa_assert(rd->defer_event == e);
+
+    mainloop->defer_enable(rd->defer_event, 0);
+    mainloop->defer_free(rd->defer_event);
+    rd->defer_event = NULL;
+
+    do_reinit(mainloop, rd);
 }
 
-static void do_reinit(pa_mainloop_api *mainloop, void *userdata) {
-    struct reinit_data *rd = userdata;
+static void do_reinit(pa_mainloop_api *mainloop, pa_restart_data *rd) {
     struct timeval tv;
 
     pa_assert_ctl_context();
@@ -66,17 +84,20 @@ static void do_reinit(pa_mainloop_api *mainloop, void *userdata) {
     /* after restart_usec, call do_init to restart the module */
     pa_gettimeofday(&tv);
     pa_timeval_add(&tv, rd->restart_usec);
-    mainloop->time_new(mainloop, &tv, call_init, rd);
+    rd->time_event = mainloop->time_new(mainloop, &tv, call_init, rd);
 }
 
-void pa_restart_module_reinit(pa_module *m, init_cb do_init, done_cb do_done, pa_usec_t restart_usec) {
-    struct reinit_data *rd;
+pa_restart_data *pa_restart_module_reinit(pa_module *m, init_cb do_init, done_cb do_done, pa_usec_t restart_usec) {
+    pa_restart_data *rd;
 
     pa_assert_ctl_context();
+    pa_assert(do_init);
+    pa_assert(do_done);
+    pa_assert(restart_usec);
 
     pa_log_info("Starting reinit for %s", m->name);
 
-    rd = pa_xnew0(struct reinit_data, 1);
+    rd = pa_xnew0(pa_restart_data, 1);
     rd->do_init = do_init;
     rd->do_done = do_done;
     rd->restart_usec = restart_usec;
@@ -84,5 +105,25 @@ void pa_restart_module_reinit(pa_module *m, init_cb do_init, done_cb do_done, pa
 
     /* defer actually doing a reinit, so that we can safely exit whatever call
      * chain we're in before we effectively reinit the module */
-    pa_mainloop_api_once(m->core->mainloop, do_reinit, rd);
+    rd->defer_event = m->core->mainloop->defer_new(m->core->mainloop, defer_callback, rd);
+    m->core->mainloop->defer_enable(rd->defer_event, 1);
+
+    return rd;
+}
+
+void pa_restart_free(pa_restart_data *rd) {
+    pa_assert_ctl_context();
+    pa_assert(rd);
+
+    if (rd->defer_event) {
+        rd->module->core->mainloop->defer_enable(rd->defer_event, 0);
+        rd->module->core->mainloop->defer_free(rd->defer_event);
+    }
+
+    if (rd->time_event) {
+        pa_log_info("Cancel reinit for %s", rd->module->name);
+        rd->module->core->mainloop->time_free(rd->time_event);
+    }
+
+    pa_xfree(rd);
 }


=====================================
src/modules/restart-module.h
=====================================
@@ -29,10 +29,20 @@ extern "C" {
 #include <pulsecore/core.h>
 #include <pulsecore/thread-mq.h>
 
+/* Init and exit callbacks of the module */
 typedef int (*init_cb)(pa_module *m);
 typedef void (*done_cb)(pa_module *m);
+/* Restart data structure */
+typedef struct pa_restart_data pa_restart_data;
 
-void pa_restart_module_reinit(pa_module *m, init_cb do_init, done_cb do_done, pa_usec_t restart_usec);
+/* Tears down the module using the done callback and schedules a restart after restart_usec.
+ * Returns a handle to the restart event. When the init callback finishes successfully during
+ * restart or when the restart should be cancelled, the restart event must be destroyed using
+ * pa_restart_free(). */
+pa_restart_data *pa_restart_module_reinit(pa_module *m, init_cb do_init, done_cb do_done, pa_usec_t restart_usec);
+
+/* Free the restart event */
+void pa_restart_free(pa_restart_data *data);
 
 #ifdef __cplusplus
 }



View it on GitLab: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/compare/0adb12e099fa2fdf4a1753f2c8d729cbb26e928b...5bba8ee621340bde1808ad42e4b61f45821ac886

-- 
View it on GitLab: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/compare/0adb12e099fa2fdf4a1753f2c8d729cbb26e928b...5bba8ee621340bde1808ad42e4b61f45821ac886
You're receiving this email because of your account on gitlab.freedesktop.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/pulseaudio-commits/attachments/20220525/58f097eb/attachment-0001.htm>


More information about the pulseaudio-commits mailing list