From d7828e117abbdad68a52af80d2eeefc35191d90a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 30 May 2018 16:28:28 +0200 Subject: [PATCH 1/5] man: fix minor typo --- man/sd_bus_add_match.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/sd_bus_add_match.xml b/man/sd_bus_add_match.xml index bb06b8fbb7..07a7b0ea17 100644 --- a/man/sd_bus_add_match.xml +++ b/man/sd_bus_add_match.xml @@ -123,7 +123,7 @@ name), from which the match string is internally generated. Optionally, these parameters may be specified as NULL in which case the relevant field of incoming signals is not tested. - sd_bus_match_signal_async() is combines the signal matching logic of + sd_bus_match_signal_async() combines the signal matching logic of sd_bus_match_signal() with the asynchronous behaviour of sd_bus_add_match_async(). From 303acb7f2db3240491abb3cd8bdba90ecfc9306d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 30 May 2018 16:29:33 +0200 Subject: [PATCH 2/5] sd-bus: add new sd_bus_slot_set_floating() call This new call allows explicit control of the "floating" state of a bus slot object. This is useful for creating a bus slot object first, retaining a reference to it, using it for making changes to the slot object (for example, set a description) and then handing it over to sd-bus for lifecycle management. It's also useful to fix #8551. --- src/libsystemd/libsystemd.sym | 2 ++ src/libsystemd/sd-bus/bus-internal.h | 8 +++++++ src/libsystemd/sd-bus/bus-slot.c | 31 ++++++++++++++++++++++++++++ src/systemd/sd-bus.h | 2 ++ 4 files changed, 43 insertions(+) diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index 529f401a84..6f2075a33a 100644 --- a/src/libsystemd/libsystemd.sym +++ b/src/libsystemd/libsystemd.sym @@ -561,4 +561,6 @@ global: sd_bus_open_with_description; sd_bus_open_user_with_description; sd_bus_open_system_with_description; + sd_bus_slot_get_floating; + sd_bus_slot_set_floating; } LIBSYSTEMD_238; diff --git a/src/libsystemd/sd-bus/bus-internal.h b/src/libsystemd/sd-bus/bus-internal.h index e84ea0f6bf..4bce0a7e32 100644 --- a/src/libsystemd/sd-bus/bus-internal.h +++ b/src/libsystemd/sd-bus/bus-internal.h @@ -128,7 +128,15 @@ struct sd_bus_slot { sd_bus *bus; void *userdata; BusSlotType type:5; + + /* Slots can be "floating" or not. If they are not floating (the usual case) then they reference the bus object + * they are associated with. This means the bus object stays allocated at least as long as there is a slot + * around associated with it. If it is floating, then the slot's lifecycle is bound to the lifecycle of the + * bus: it will be disconnected from the bus when the bus is destroyed, and it keeping the slot reffed hence + * won't mean the bus stays reffed too. Internally this means the reference direction is reversed: floating + * slots objects are referenced by the bus object, and not vice versa. */ bool floating:1; + bool match_added:1; char *description; diff --git a/src/libsystemd/sd-bus/bus-slot.c b/src/libsystemd/sd-bus/bus-slot.c index 3745426c5c..3e592e3a2f 100644 --- a/src/libsystemd/sd-bus/bus-slot.c +++ b/src/libsystemd/sd-bus/bus-slot.c @@ -260,6 +260,37 @@ _public_ void* sd_bus_slot_get_current_userdata(sd_bus_slot *slot) { return slot->bus->current_userdata; } +_public_ int sd_bus_slot_get_floating(sd_bus_slot *slot) { + assert_return(slot, -EINVAL); + + return slot->floating; +} + +_public_ int sd_bus_slot_set_floating(sd_bus_slot *slot, int b) { + assert_return(slot, -EINVAL); + + if (slot->floating == !!b) + return 0; + + if (!slot->bus) /* already disconnected slots can't be reconnected */ + return -ESTALE; + + slot->floating = b; + + /* When a slot is "floating" then the bus references the slot. Otherwise the slot references the bus. Hence, + * when we move from one to the other, let's increase one reference and decrease the other. */ + + if (b) { + sd_bus_slot_ref(slot); + sd_bus_unref(slot->bus); + } else { + sd_bus_ref(slot->bus); + sd_bus_slot_unref(slot); + } + + return 1; +} + _public_ int sd_bus_slot_set_description(sd_bus_slot *slot, const char *description) { assert_return(slot, -EINVAL); diff --git a/src/systemd/sd-bus.h b/src/systemd/sd-bus.h index 90b9dd84c2..740e40439e 100644 --- a/src/systemd/sd-bus.h +++ b/src/systemd/sd-bus.h @@ -228,6 +228,8 @@ void *sd_bus_slot_get_userdata(sd_bus_slot *slot); void *sd_bus_slot_set_userdata(sd_bus_slot *slot, void *userdata); int sd_bus_slot_set_description(sd_bus_slot *slot, const char *description); int sd_bus_slot_get_description(sd_bus_slot *slot, const char **description); +int sd_bus_slot_get_floating(sd_bus_slot *slot); +int sd_bus_slot_set_floating(sd_bus_slot *slot, int b); sd_bus_message* sd_bus_slot_get_current_message(sd_bus_slot *slot); sd_bus_message_handler_t sd_bus_slot_get_current_handler(sd_bus_slot *bus); From 1004b2c7bc5adb8dba23e209c88f74a02ac5f709 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 30 May 2018 16:33:12 +0200 Subject: [PATCH 3/5] man: document the new sd_bus_slot_set_floating() call Also extend the memory management description of sd-bus highlighting the effect of "floating" slot objects a bit. --- man/rules/meson.build | 1 + man/sd_bus_add_match.xml | 7 +- man/sd_bus_default.xml | 35 +++++---- man/sd_bus_slot_set_floating.xml | 118 +++++++++++++++++++++++++++++++ 4 files changed, 144 insertions(+), 17 deletions(-) create mode 100644 man/sd_bus_slot_set_floating.xml diff --git a/man/rules/meson.build b/man/rules/meson.build index d3ed9f8a6a..0fb7978c21 100644 --- a/man/rules/meson.build +++ b/man/rules/meson.build @@ -231,6 +231,7 @@ manpages = [ ['sd_bus_set_connected_signal', '3', ['sd_bus_get_connected_signal'], ''], ['sd_bus_set_sender', '3', ['sd_bus_get_sender'], ''], ['sd_bus_set_watch_bind', '3', ['sd_bus_get_watch_bind'], ''], + ['sd_bus_slot_set_floating', '3', ['sd_bus_slot_get_floating'], ''], ['sd_bus_track_add_name', '3', ['sd_bus_track_add_sender', diff --git a/man/sd_bus_add_match.xml b/man/sd_bus_add_match.xml index 07a7b0ea17..13f2b06af0 100644 --- a/man/sd_bus_add_match.xml +++ b/man/sd_bus_add_match.xml @@ -132,7 +132,9 @@ at a later time with sd_bus_slot_unref3. If specified as NULL the lifetime of the match is bound to the lifetime of the bus object itself, and the - match cannot be removed independently. + match is generally not removed independently. See + sd_bus_slot_set_floating3 for + details. The message m passed to the callback is only borrowed, that is, the callback should not call sd_bus_message_unref3 @@ -175,7 +177,8 @@ sd-bus3, sd_bus_slot_unref3, sd_bus_message_ref3, - sd_bus_set_bus_client3 + sd_bus_set_bus_client3, + sd_bus_slot_set_floating3 diff --git a/man/sd_bus_default.xml b/man/sd_bus_default.xml index f36272e854..da06fb9cbf 100644 --- a/man/sd_bus_default.xml +++ b/man/sd_bus_default.xml @@ -206,21 +206,26 @@ sd_bus_unref3 to drop the reference. - Queued but unwritten/unread messages also keep a reference - to their bus connection object. For this reason, even if an - application dropped all references to a bus connection, it might - not get destroyed right away. Until all incoming queued - messages are read, and until all outgoing unwritten messages are - written, the bus object will stay - alive. sd_bus_flush() may be used to write - all outgoing queued messages so they drop their references. To - flush the unread incoming messages, use - sd_bus_close(), which will also close the bus - connection. When using the default bus logic, it is a good idea to - first invoke sd_bus_flush() followed by - sd_bus_close() when a thread or process - terminates, and thus its bus connection object should be - freed. + Queued but unwritten/unread messages keep a reference to their bus connection object. For this reason, even + if an application dropped all references to a bus connection, it might not get destroyed right away. Until all + incoming queued messages are read, and until all outgoing unwritten messages are written, the bus object will stay + alive. sd_bus_flush() may be used to write all outgoing queued messages so they drop their + references. To flush the unread incoming messages, use sd_bus_close(), which will also close + the bus connection. When using the default bus logic, it is a good idea to first invoke + sd_bus_flush() followed by sd_bus_close() when a thread or process + terminates, and thus its bus connection object should be freed. + + Normally, slot objects (as created by + sd_bus_add_match3 and similar + calls) keep a reference to their bus connection object, too. Thus, as long as a bus slot object remains referenced + its bus object will remain allocated too. Optionally, bus slot objects may be placed in "floating" mode. When in + floating mode the life cycle of the bus slot object is bound to the bus object, i.e. when the bus object is freed + the bus slot object is automatically unreferenced too. The floating state of a slot object may be controlled + explicitly with + sd_bus_slot_set_floating3, + though usually floating bus slot objects are created by passing NULL as the + slot parameter of sd_bus_add_match() and related calls, thus indicating + that the caller is not directly interested in referencing and managing the bus slot object. The life cycle of the default bus connection should be the responsibility of the code that creates/owns the thread the diff --git a/man/sd_bus_slot_set_floating.xml b/man/sd_bus_slot_set_floating.xml new file mode 100644 index 0000000000..2ac5090193 --- /dev/null +++ b/man/sd_bus_slot_set_floating.xml @@ -0,0 +1,118 @@ + + + + + + + + + sd_bus_slot_set_floating + systemd + + + + sd_bus_slot_set_floating + 3 + + + + sd_bus_slot_set_floating + sd_bus_slot_get_floating + + Control whether a bus slot object is "floating". + + + + + #include <systemd/sd-bus.h> + + + int sd_bus_slot_set_floating + sd_bus_slot *bus + int b + + + + int sd_bus_slot_get_floating + sd_bus_slot *slot + + + + + + + Description + + sd_bus_slot_set_floating() controls whether the specified bus slot object + slot shall be "floating" or not. A floating bus slot object's lifetime is bound to the + lifetime of the bus object it is associated with, meaning that it remains allocated as long as the bus object + itself and is freed automatically when the bus object is freed. Regular (i.e. non-floating) bus slot objects keep + the bus referenced, hence the bus object remains allocated at least as long as there remains at least one + referenced bus slot object around. The floating state hence controls the direction of referencing between the bus + object and the bus slot objects: if floating the bus pins the the bus slot, and otherwise the bus slot pins the bus + objects. Use sd_bus_slot_set_floating() to switch between both modes: if the + b parameter is zero, the slot object is considered floating, otherwise it is made a regular + (non-floating) slot object. + + Bus slot objects may be allocated with calls such as + sd_bus_add_match3. If the + slot of these functions is non-NULL the slot object will be of the + regular kind (i.e. non-floating), otherwise it will be created floating. With + sd_bus_slot_set_floating() a bus slot object allocated as regular can be converted into a + floating object and back. This is particularly useful for creating a bus slot object, then changing parameters of + it, and then turning it into a floating object, whose lifecycle is managed by the bus object. + + sd_bus_slot_get_floating() returns the current floating state of the specified bus slot + object. It returns negative on error, zero if the bus slot object is a regular (non-floating) object and positive + otherwise. + + + + Return Value + + On success, these functions return 0 or a positive integer. On failure, they return a negative errno-style + error code. + + + + Errors + + Returned errors may indicate the following problems: + + + + -ECHILD + + The bus connection has been created in a different process. + + + + -ESTALE + + The bus object the specified bus slot object is associated with has already been freed, and + hence no change in the floating state can be made anymore. + + + + + + Notes + + sd_bus_slot_set_floating() and sd_bus_slot_get_floating() are + available as a shared library, which can be compiled and linked to with the + libsystemd pkg-config1 file. + + + + See Also + + + systemd1, + sd-bus3, + sd_bus_add_match3 + + + + From 7ae497b936998af6b16f746bd9695d16488b50cc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 30 May 2018 16:34:06 +0200 Subject: [PATCH 4/5] bus-slot: for bus slot objects with no explicit description use the match string as description Let's make debugging a but easier with implicit descriptions for some match objects. --- src/libsystemd/sd-bus/bus-slot.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-slot.c b/src/libsystemd/sd-bus/bus-slot.c index 3e592e3a2f..34cfdd29e9 100644 --- a/src/libsystemd/sd-bus/bus-slot.c +++ b/src/libsystemd/sd-bus/bus-slot.c @@ -300,8 +300,13 @@ _public_ int sd_bus_slot_set_description(sd_bus_slot *slot, const char *descript _public_ int sd_bus_slot_get_description(sd_bus_slot *slot, const char **description) { assert_return(slot, -EINVAL); assert_return(description, -EINVAL); - assert_return(slot->description, -ENXIO); - *description = slot->description; + if (slot->description) + *description = slot->description; + else if (slot->type == BUS_MATCH_CALLBACK) + *description = slot->match_callback.match_string; + else + return -ENXIO; + return 0; } From fc2d4c89b87a224a9dd06756db9b964bb5170af0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 30 May 2018 16:35:36 +0200 Subject: [PATCH 5/5] sd-bus: make add match method callback slot "floating" When we allocate an asynchronous match object we will allocate an asynchronous bus call object to install the match server side. Previously the call slot would be created as regular slot, i.e. non-floating which meant installing the match even if it was itself floating would result in a non-floating slot to be created internally, which ultimately would mean the sd_bus object would be referenced by it, and thus never be freed. Let's fix that by making the match method callback floating in any case as we have no interest in leaving the bus allocated beyond the match slot. Fixes: #8551 --- src/libsystemd/sd-bus/sd-bus.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index c1974dabe7..8ecb381586 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -3227,13 +3227,21 @@ static int bus_add_match_full( goto finish; } - if (asynchronous) + if (asynchronous) { r = bus_add_match_internal_async(bus, &s->match_callback.install_slot, s->match_callback.match_string, add_match_callback, s); - else + + if (r < 0) + return r; + + /* Make the slot of the match call floating now. We need the reference, but we don't + * want that this match pins the bus object, hence we first create it non-floating, but + * then make it floating. */ + r = sd_bus_slot_set_floating(s->match_callback.install_slot, true); + } else r = bus_add_match_internal(bus, s->match_callback.match_string); if (r < 0) goto finish;