[pulseaudio-commits] [SCM] PulseAudio Sound Server branch, master, updated. v1.0-dev-221-g8dc0df0

Colin Guthrie gitmailer-noreply at 0pointer.de
Sat Mar 26 04:11:12 PDT 2011


This is an automated email from the git hooks/post-receive script. It was
generated because of a push to the "PulseAudio Sound Server" repository.

The master branch has been updated
      from  f49711c99a86075de0a2337c4ea71049a9aa146a (commit)

- Log -----------------------------------------------------------------
8dc0df0 bluetooth: Fix a double-free-esque error introduced in 8f3ef04b
1e381fb dbus: Do not refcnt the core.
7aa8a3f daemon: Fix regression introduced in f1d1447e.
34ddc5b daemon: Fix some more error paths in the double forking.
-----------------------------------------------------------------------

Summary of changes:
 src/daemon/main.c                      |   55 +++++++++++++++++++++++++++++--
 src/modules/bluetooth/bluetooth-util.c |    2 -
 src/modules/dbus/iface-core.c          |    3 +-
 src/modules/dbus/iface-memstats.c      |    3 +-
 src/pulsecore/protocol-dbus.c          |    4 +--
 5 files changed, 54 insertions(+), 13 deletions(-)

-----------------------------------------------------------------------

commit 34ddc5b9c533ed83d53238fff0777663f9c50336
Author: Colin Guthrie <cguthrie at mandriva.org>
Date:   Fri Mar 25 09:12:51 2011 +0000

    daemon: Fix some more error paths in the double forking.
    
    As spotted by Tanu Kaskinen:
    
    The first process: daemon_pipe is not closed if the first fork() call
    fails. Even if it doesn't fail, the first process never closes
    daemon_pipe[0].
    
    The second process: daemon_pipe[1] is not closed if anything fails
    between the first and the second fork() call. Also, if the second fork
    fails, then the finish section writes to daemon_pipe2[1], even though
    only the third process should do that. Also, if anything fails between
    the first and the second fork, then the second process never writes
    anything to daemon_pipe[1]. I don't know what happens in the first
    process in this case - does it get an error or does pa_loop_read() get
    stuck.
    
    The third process: No problems :)

diff --git a/src/daemon/main.c b/src/daemon/main.c
index b77a8b6..f939313 100644
--- a/src/daemon/main.c
+++ b/src/daemon/main.c
@@ -729,6 +729,7 @@ int main(int argc, char *argv[]) {
 
         if ((child = fork()) < 0) {
             pa_log(_("fork() failed: %s"), pa_cstrerror(errno));
+            pa_close_pipe(daemon_pipe);
             goto finish;
         }
 
@@ -793,6 +794,7 @@ int main(int argc, char *argv[]) {
 
         if ((child = fork()) < 0) {
             pa_log(_("fork() failed: %s"), pa_cstrerror(errno));
+            pa_close_pipe(daemon_pipe2);
             goto finish;
         }
 
@@ -1128,10 +1130,15 @@ finish:
     pa_signal_done();
 
 #ifdef HAVE_FORK
-    if (daemon_pipe2[1] >= 0)
+    /* If we have daemon_pipe[1] still open, this means we've failed after
+     * the first fork, but before the second. Therefore just write to it. */
+    if (daemon_pipe[1] >= 0)
+        pa_loop_write(daemon_pipe[1], &retval, sizeof(retval), NULL);
+    else if (daemon_pipe2[1] >= 0)
         pa_loop_write(daemon_pipe2[1], &retval, sizeof(retval), NULL);
 
     pa_close_pipe(daemon_pipe2);
+    pa_close_pipe(daemon_pipe);
 #endif
 
     if (mainloop)

commit 7aa8a3fa8015d17240e5fc27bf44eb7d22e7e13a
Author: Colin Guthrie <cguthrie at mandriva.org>
Date:   Fri Mar 25 23:03:31 2011 +0000

    daemon: Fix regression introduced in f1d1447e.
    
    With Tanu's patch, the server no longer starts when a server is configured.
    While this is sensible in most circumstances there is a corner case where
    we still want to start.
    
    In a typical X11 login, module-x11-publish will be loaded and will thus
    set the PULSE_SERVER X11 property on the root window. This then hits the
    check introduced in f1d1447e and exits. If PA had previously crashed
    (thus leaving behind it's X11 properties) then this means that we will not
    autospawn nor even allow ourselves to be started manually until
    pax11publish -r is run to clear out the X11 properties. This is obviously
    not desirable.
    
    This patch introduces a more in-depth check of the server. If it looks like
    a local unix domain socket, then we do not exit straight away and instead
    probe further. This should not pose any problems with e.g. remote SSH
    usage as the DBus Machine ID is used in the server string.

diff --git a/src/daemon/main.c b/src/daemon/main.c
index f939313..5316656 100644
--- a/src/daemon/main.c
+++ b/src/daemon/main.c
@@ -90,6 +90,7 @@
 #include <pulsecore/once.h>
 #include <pulsecore/shm.h>
 #include <pulsecore/memtrap.h>
+#include <pulsecore/strlist.h>
 #ifdef HAVE_DBUS
 #include <pulsecore/dbus-shared.h>
 #endif
@@ -673,10 +674,49 @@ int main(int argc, char *argv[]) {
     }
 
     if (conf->cmd == PA_CMD_START && (configured_address = check_configured_address())) {
-        pa_log_notice(_("User-configured server at %s, not autospawning."), configured_address);
+        /* There is an server address in our config, but where did it come from?
+         * By default a standard X11 login will load module-x11-publish which will
+         * inject PULSE_SERVER X11 property. If the PA daemon crashes, we will end
+         * up hitting this code path. So we have to check to see if our configured_address
+         * is the same as the value that would go into this property so that we can
+         * recover (i.e. autospawn) from a crash.
+         */
+        char *ufn;
+        pa_bool_t start_anyway = FALSE;
+
+        if ((ufn = pa_runtime_path(PA_NATIVE_DEFAULT_UNIX_SOCKET))) {
+            char *id;
+
+            if ((id = pa_machine_id())) {
+                pa_strlist *server_list;
+                char formatted_ufn[256];
+
+                pa_snprintf(formatted_ufn, sizeof(formatted_ufn), "{%s}unix:%s", id, ufn);
+                pa_xfree(id);
+
+                if ((server_list = pa_strlist_parse(configured_address))) {
+                    char *u = NULL;
+
+                    /* We only need to check the first server */
+                    server_list = pa_strlist_pop(server_list, &u);
+                    pa_strlist_free(server_list);
+
+                    start_anyway = (u && pa_streq(formatted_ufn, u));
+                    pa_xfree(u);
+                }
+            }
+            pa_xfree(ufn);
+        }
+
+        if (!start_anyway) {
+            pa_log_notice(_("User-configured server at %s, refusing to start/autospawn."), configured_address);
+            pa_xfree(configured_address);
+            retval = 0;
+            goto finish;
+        }
+
+        pa_log_notice(_("User-configured server at %s, which appears to be local. Probing deeper."), configured_address);
         pa_xfree(configured_address);
-        retval = 0;
-        goto finish;
     }
 
     if (conf->system_instance && !conf->disallow_exit)

commit 1e381fbffc190fdede27d6f27a2d113daf3e791d
Author: Colin Guthrie <cguthrie at mandriva.org>
Date:   Fri Mar 25 23:43:26 2011 +0000

    dbus: Do not refcnt the core.
    
    We should not call pa_core_ref() anywhere in the code. Doing so
    will prevent proper daemon shutdown as the only call (in daemon/main.c)
    to pa_core_unref() should always call free_core() and perform a normal
    shutdown (i.e. unload all modules gracefully).

diff --git a/src/modules/dbus/iface-core.c b/src/modules/dbus/iface-core.c
index 268a87e..bb43df9 100644
--- a/src/modules/dbus/iface-core.c
+++ b/src/modules/dbus/iface-core.c
@@ -1975,7 +1975,7 @@ pa_dbusiface_core *pa_dbusiface_core_new(pa_core *core) {
     pa_assert(core);
 
     c = pa_xnew(pa_dbusiface_core, 1);
-    c->core = pa_core_ref(core);
+    c->core = core;
     c->subscription = pa_subscription_new(core, PA_SUBSCRIPTION_MASK_ALL, subscription_cb, c);
     c->dbus_protocol = pa_dbus_protocol_get(core);
     c->cards = pa_hashmap_new(pa_idxset_trivial_hash_func, pa_idxset_trivial_compare_func);
@@ -2124,7 +2124,6 @@ void pa_dbusiface_core_free(pa_dbusiface_core *c) {
         pa_source_unref(c->fallback_source);
 
     pa_dbus_protocol_unref(c->dbus_protocol);
-    pa_core_unref(c->core);
 
     pa_xfree(c);
 }
diff --git a/src/modules/dbus/iface-memstats.c b/src/modules/dbus/iface-memstats.c
index 73a84be..4cd692d 100644
--- a/src/modules/dbus/iface-memstats.c
+++ b/src/modules/dbus/iface-memstats.c
@@ -202,7 +202,7 @@ pa_dbusiface_memstats *pa_dbusiface_memstats_new(pa_dbusiface_core *dbus_core, p
     pa_assert(core);
 
     m = pa_xnew(pa_dbusiface_memstats, 1);
-    m->core = pa_core_ref(core);
+    m->core = core;
     m->path = pa_sprintf_malloc("%s/%s", PA_DBUS_CORE_OBJECT_PATH, OBJECT_NAME);
     m->dbus_protocol = pa_dbus_protocol_get(core);
 
@@ -219,7 +219,6 @@ void pa_dbusiface_memstats_free(pa_dbusiface_memstats *m) {
     pa_xfree(m->path);
 
     pa_dbus_protocol_unref(m->dbus_protocol);
-    pa_core_unref(m->core);
 
     pa_xfree(m);
 }
diff --git a/src/pulsecore/protocol-dbus.c b/src/pulsecore/protocol-dbus.c
index 13c05d9..c329915 100644
--- a/src/pulsecore/protocol-dbus.c
+++ b/src/pulsecore/protocol-dbus.c
@@ -125,7 +125,7 @@ static pa_dbus_protocol *dbus_protocol_new(pa_core *c) {
 
     p = pa_xnew(pa_dbus_protocol, 1);
     PA_REFCNT_INIT(p);
-    p->core = pa_core_ref(c);
+    p->core = c;
     p->objects = pa_hashmap_new(pa_idxset_string_hash_func, pa_idxset_string_compare_func);
     p->connections = pa_hashmap_new(pa_idxset_trivial_hash_func, pa_idxset_trivial_compare_func);
     p->extensions = pa_idxset_new(pa_idxset_string_hash_func, pa_idxset_string_compare_func);
@@ -178,8 +178,6 @@ void pa_dbus_protocol_unref(pa_dbus_protocol *p) {
 
     pa_assert_se(pa_shared_remove(p->core, "dbus-protocol") >= 0);
 
-    pa_core_unref(p->core);
-
     pa_xfree(p);
 }
 

commit 8dc0df05383e12bb42511f5a732ee636d1a66603
Author: Colin Guthrie <cguthrie at mandriva.org>
Date:   Sat Mar 26 00:12:09 2011 +0000

    bluetooth: Fix a double-free-esque error introduced in 8f3ef04b

diff --git a/src/modules/bluetooth/bluetooth-util.c b/src/modules/bluetooth/bluetooth-util.c
index f1c7c4c..5d388ce 100644
--- a/src/modules/bluetooth/bluetooth-util.c
+++ b/src/modules/bluetooth/bluetooth-util.c
@@ -1565,8 +1565,6 @@ void pa_bluetooth_discovery_unref(pa_bluetooth_discovery *y) {
         if (y->filter_added)
             dbus_connection_remove_filter(pa_dbus_connection_get(y->connection), filter_cb, y);
 
-        dbus_connection_remove_filter(pa_dbus_connection_get(y->connection), filter_cb, y);
-
         pa_dbus_connection_unref(y->connection);
     }
 

-- 
hooks/post-receive
PulseAudio Sound Server



More information about the pulseaudio-commits mailing list