[systemd-commits] 3 commits - src/bus-proxyd src/libsystemd
Lennart Poettering
lennart at kemper.freedesktop.org
Wed Jul 2 08:45:07 PDT 2014
src/bus-proxyd/bus-proxyd.c | 8 ++++-
src/libsystemd/sd-bus/bus-message.c | 5 ++-
src/libsystemd/sd-bus/sd-bus.c | 54 ------------------------------------
3 files changed, 11 insertions(+), 56 deletions(-)
New commits:
commit 7bb4d371af5ec6b8c50b71d2a80c2866d8134d9a
Author: Lennart Poettering <lennart at poettering.net>
Date: Wed Jul 2 17:36:47 2014 +0200
sd-bus: when an event loop terminates, explicitly close the bus
This makes sure we actually release the bus and all the messages it
references.
diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c
index d52afe8..eb267d4 100644
--- a/src/libsystemd/sd-bus/sd-bus.c
+++ b/src/libsystemd/sd-bus/sd-bus.c
@@ -2940,6 +2940,7 @@ static int quit_callback(sd_event_source *event, void *userdata) {
assert(event);
sd_bus_flush(bus);
+ sd_bus_close(bus);
return 1;
}
commit b5eca3a2059f9399d1dc52cbcf9698674c4b1cf0
Author: Lennart Poettering <lennart at poettering.net>
Date: Wed Jul 2 17:29:09 2014 +0200
bus: drop bus/message GC logic
When a caller drops all references to a bus and its messages while the
messages where still queue, this causes the bus to reference the
messages, and the messages to reference the bus, without anybody else
keeping a reference, which is something we so far considered a leak, and
tried to fix with a GC logic that would recognize cases like this, and
drop the reference.
This GC logic has been broken sofar, and remained unfixed. This commit
removes it altogther, replacing it with nothing. The rationale is that
simply because all refs to the bus have been dropped its queued messages
should *still* be written to the bus, even if the caller doesn't retain
any reference to either bus nor message. This means it was actually
wrong to attempt to clean up the bus in this case.
The proper way how applications should handle this is by explicitly
invoking sd_bus_close(), when they want busses to go away. This is
probably want they want to do anyway to avoid getting spurious
callbacks after they stopped using a bus.
diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c
index eaffa2d..4768a1f 100644
--- a/src/libsystemd/sd-bus/bus-message.c
+++ b/src/libsystemd/sd-bus/bus-message.c
@@ -801,9 +801,10 @@ _public_ sd_bus_message* sd_bus_message_unref(sd_bus_message *m) {
assert(m->n_ref > 0);
m->n_ref--;
- if (m->n_ref <= 0)
- message_free(m);
+ if (m->n_ref > 0)
+ return NULL;
+ message_free(m);
return NULL;
}
diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c
index 1f1a4d3..d52afe8 100644
--- a/src/libsystemd/sd-bus/sd-bus.c
+++ b/src/libsystemd/sd-bus/sd-bus.c
@@ -75,12 +75,6 @@ static void bus_close_fds(sd_bus *b) {
static void bus_reset_queues(sd_bus *b) {
assert(b);
- /* NOTE: We _must_ decrement b->Xqueue_size before calling
- * sd_bus_message_unref() for _each_ message. Otherwise the
- * self-reference checks in sd_bus_unref() will fire for each message.
- * We would thus recurse into sd_bus_message_unref() and trigger the
- * assert(m->n_ref > 0) */
-
while (b->rqueue_size > 0)
sd_bus_message_unref(b->rqueue[--b->rqueue_size]);
@@ -1364,53 +1358,6 @@ _public_ sd_bus *sd_bus_unref(sd_bus *bus) {
if (!bus)
return NULL;
- /* TODO/FIXME: It's naive to think REFCNT_GET() is thread-safe in any
- * way but exclusive REFCNT_DEC(). The current logic _must_ lock around
- * REFCNT_GET() until REFCNT_DEC() or two threads might end up in
- * parallel in bus_reset_queues(). But locking would totally break the
- * recursion we introduce by bus_reset_queues()...
- * (Imagine one thread in sd_bus_message_unref() setting n_ref to 0 and
- * thus calling into sd_bus_unref(). If at the same time the real
- * thread calls sd_bus_unref(), both end up with "q == true" and will
- * call into bus_reset_queues().
- * If we require the main bus to be alive until all dispatch threads
- * are done, there is no need to do ref-counts at all. So in both ways,
- * the REFCNT thing is humbug.)
- *
- * On a second note: messages are *not* required to have ->bus set nor
- * does it have to be _this_ bus that they're assigned to. This whole
- * ref-cnt checking breaks apart if a message is not assigned to us.
- * (which is _very_ easy to trigger with the current API). */
-
- if (REFCNT_GET(bus->n_ref) == bus->rqueue_size + bus->wqueue_size + 1) {
- bool q = true;
-
- for (i = 0; i < bus->rqueue_size; i++)
- if (bus->rqueue[i]->n_ref > 1) {
- q = false;
- break;
- }
-
- if (q) {
- for (i = 0; i < bus->wqueue_size; i++)
- if (bus->wqueue[i]->n_ref > 1) {
- q = false;
- break;
- }
- }
-
- /* We are the only holders on the messages, and the
- * messages are the only holders on us, so let's drop
- * the messages and thus implicitly also kill our own
- * last references.
- * bus_reset_queues() decrements the queue-size before
- * calling into sd_bus_message_unref(). Thus, it
- * protects us from recursion. */
-
- if (q)
- bus_reset_queues(bus);
- }
-
i = REFCNT_DEC(bus->n_ref);
if (i > 0)
return NULL;
commit 62bb05f64fe4d7aeadffb4815ba6a9082b1da285
Author: Lennart Poettering <lennart at poettering.net>
Date: Wed Jul 2 17:12:24 2014 +0200
bus-proxy: restore operation in non-kdbus mode
bus-proxyd is not only the bridge between legacy dbus clients and kdbus
but is also used to access remote dbus servers via ssh. Let's make sure
it actually works for that.
diff --git a/src/bus-proxyd/bus-proxyd.c b/src/bus-proxyd/bus-proxyd.c
index 07995ec..3f095e0 100644
--- a/src/bus-proxyd/bus-proxyd.c
+++ b/src/bus-proxyd/bus-proxyd.c
@@ -47,7 +47,7 @@
#include "capability.h"
#include "bus-policy.h"
-static const char *arg_address = KERNEL_SYSTEM_BUS_PATH;
+static const char *arg_address = DEFAULT_SYSTEM_BUS_PATH;
static char *arg_command_line_buffer = NULL;
static bool arg_drop_privileges = false;
static char **arg_configuration = NULL;
@@ -281,6 +281,9 @@ static int process_policy(sd_bus *a, sd_bus *b, sd_bus_message *m) {
assert(b);
assert(m);
+ if (!a->is_kernel)
+ return 0;
+
if (!sd_bus_message_is_method_call(m, "org.freedesktop.DBus.Properties", "GetAll"))
return 0;
@@ -467,6 +470,9 @@ static int process_driver(sd_bus *a, sd_bus *b, sd_bus_message *m) {
assert(b);
assert(m);
+ if (!a->is_kernel)
+ return 0;
+
if (!streq_ptr(sd_bus_message_get_destination(m), "org.freedesktop.DBus"))
return 0;
More information about the systemd-commits
mailing list