summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorDavid Herrmann <dh.herrmann@gmail.com>2014-03-22 19:31:31 +0100
committerDavid Herrmann <dh.herrmann@gmail.com>2014-03-22 19:35:25 +0100
commit374c356979ba7222fa7e09005824fe6996b0e91e (patch)
treec6498deae329785c2eb1ebe6766e392ecfe8a5d5 /src
parent23aedd029452b65c11b6eb838e44f60be1c66814 (diff)
sd-bus: mark sd_bus_unref() as broken regarding self-refs
If you allocate a message with bus==NULL and then unref the main bus, it will free your message underneath and your program will go boom! To fix that, we really need to figure out what the semantics for self-references (m->bus) should be and when/where/what accesses are actually allowed. Same is true for the pseudo-thread-safety we employ..
Diffstat (limited to 'src')
-rw-r--r--src/libsystemd/sd-bus/sd-bus.c18
1 files changed, 18 insertions, 0 deletions
diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c
index 984611fd4d..bbe61a6a8f 100644
--- a/src/libsystemd/sd-bus/sd-bus.c
+++ b/src/libsystemd/sd-bus/sd-bus.c
@@ -1394,6 +1394,24 @@ _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;