From 42541a71a2c9560fe37fbef37b868bb53b53aef4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Fri, 1 Mar 2019 12:43:12 +0100
Subject: [PATCH] bus: make reference counting non-atomic

We had atomic counters, but all other operations were non-serialized. This
means that concurrent access to the bus object was only safe if _all_ threads
were doing read-only access. Even sending of messages from threads would not be
possible, because after sending of the message we usually want to remove it
from the send queue in the bus object, which would race. Let's just kill this.
---
 src/libsystemd/sd-bus/bus-internal.h     | 11 +----------
 src/libsystemd/sd-bus/sd-bus.c           |  4 ++--
 src/libsystemd/sd-bus/test-bus-cleanup.c | 17 ++++++++---------
 3 files changed, 11 insertions(+), 21 deletions(-)

diff --git a/src/libsystemd/sd-bus/bus-internal.h b/src/libsystemd/sd-bus/bus-internal.h
index 04864d4df0..63f1404758 100644
--- a/src/libsystemd/sd-bus/bus-internal.h
+++ b/src/libsystemd/sd-bus/bus-internal.h
@@ -13,7 +13,6 @@
 #include "hashmap.h"
 #include "list.h"
 #include "prioq.h"
-#include "refcnt.h"
 #include "socket-util.h"
 #include "util.h"
 
@@ -171,15 +170,7 @@ enum bus_auth {
 };
 
 struct sd_bus {
-        /* We use atomic ref counting here since sd_bus_message
-           objects retain references to their originating sd_bus but
-           we want to allow them to be processed in a different
-           thread. We won't provide full thread safety, but only the
-           bare minimum that makes it possible to use sd_bus and
-           sd_bus_message objects independently and on different
-           threads as long as each object is used only once at the
-           same time. */
-        RefCount n_ref;
+        unsigned n_ref;
 
         enum bus_state state;
         int input_fd, output_fd;
diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c
index 69f1519976..ba791db4f2 100644
--- a/src/libsystemd/sd-bus/sd-bus.c
+++ b/src/libsystemd/sd-bus/sd-bus.c
@@ -236,7 +236,7 @@ _public_ int sd_bus_new(sd_bus **ret) {
                 return -ENOMEM;
 
         *b = (sd_bus) {
-                .n_ref = REFCNT_INIT,
+                .n_ref = 1,
                 .input_fd = -1,
                 .output_fd = -1,
                 .inotify_fd = -1,
@@ -1585,7 +1585,7 @@ void bus_enter_closing(sd_bus *bus) {
         bus_set_state(bus, BUS_CLOSING);
 }
 
-DEFINE_PUBLIC_ATOMIC_REF_UNREF_FUNC(sd_bus, sd_bus, bus_free);
+DEFINE_PUBLIC_TRIVIAL_REF_UNREF_FUNC(sd_bus, sd_bus, bus_free);
 
 _public_ int sd_bus_is_open(sd_bus *bus) {
         assert_return(bus, -EINVAL);
diff --git a/src/libsystemd/sd-bus/test-bus-cleanup.c b/src/libsystemd/sd-bus/test-bus-cleanup.c
index bea722ba06..99d335e3fc 100644
--- a/src/libsystemd/sd-bus/test-bus-cleanup.c
+++ b/src/libsystemd/sd-bus/test-bus-cleanup.c
@@ -7,7 +7,6 @@
 #include "bus-internal.h"
 #include "bus-message.h"
 #include "bus-util.h"
-#include "refcnt.h"
 #include "tests.h"
 
 static bool use_system_bus = false;
@@ -16,7 +15,7 @@ static void test_bus_new(void) {
         _cleanup_(sd_bus_unrefp) sd_bus *bus = NULL;
 
         assert_se(sd_bus_new(&bus) == 0);
-        printf("after new: refcount %u\n", REFCNT_GET(bus->n_ref));
+        assert_se(bus->n_ref == 1);
 }
 
 static int test_bus_open(void) {
@@ -32,7 +31,7 @@ static int test_bus_open(void) {
         }
 
         assert_se(r >= 0);
-        printf("after open: refcount %u\n", REFCNT_GET(bus->n_ref));
+        assert_se(bus->n_ref >= 1); /* we send a hello message when opening, so the count is above 1 */
 
         return 0;
 }
@@ -45,10 +44,10 @@ static void test_bus_new_method_call(void) {
 
         assert_se(sd_bus_message_new_method_call(bus, &m, "a.service.name", "/an/object/path", "an.interface.name", "AMethodName") >= 0);
 
-        printf("after message_new_method_call: refcount %u\n", REFCNT_GET(bus->n_ref));
-
+        assert_se(m->n_ref == 1); /* We hold the only reference to the message */
+        assert_se(bus->n_ref >= 2);
         sd_bus_flush_close_unref(bus);
-        printf("after bus_flush_close_unref: refcount %u\n", m->n_ref);
+        assert_se(m->n_ref == 1);
 }
 
 static void test_bus_new_signal(void) {
@@ -59,10 +58,10 @@ static void test_bus_new_signal(void) {
 
         assert_se(sd_bus_message_new_signal(bus, &m, "/an/object/path", "an.interface.name", "Name") >= 0);
 
-        printf("after message_new_signal: refcount %u\n", REFCNT_GET(bus->n_ref));
-
+        assert_se(m->n_ref == 1); /* We hold the only reference to the message */
+        assert_se(bus->n_ref >= 2);
         sd_bus_flush_close_unref(bus);
-        printf("after bus_flush_close_unref: refcount %u\n", m->n_ref);
+        assert_se(m->n_ref == 1);
 }
 
 int main(int argc, char **argv) {