bus: change constructor semantics
Previously, the bus object had no semantics to recognize if
it was the primary-holder of a sd_bus or a secondary one.
On destruction, the object always called 'sd_bus_unref' and it
was assumed that the sd-bus library would properly close the
bus on last-unref. This turns out not to be true and lead to
file-descriptor leaks.
Change the constructor to mimic what is done in 'message':
- When the constructor is called with a sd_bus*, the class becomes
a secondary-holder and calls 'sd_bus_unref' on destruction, which
is the old behavior. The class also calls 'sd_bus_ref' on
construction, so this should no longer be done by callers.
- When the constructor is called via the new_* functions (or by
adding a std::false_type parameter), the class becomes a primary-
holder and calls 'sd_bus_flush_close_unref' on destruction.
Note: Current callers that use syntax like 'bus(sd_bus_ref(b))' will
need to be updated to avoid a file-descriptor leak.
Fixes openbmc/openbmc#1432.
Change-Id: Ic0c582f8fbfd44775bcdaffa6a21518a0cd056b1
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
diff --git a/sdbusplus/bus.hpp.in b/sdbusplus/bus.hpp.in
index 94d5588..185ffd7 100644
--- a/sdbusplus/bus.hpp.in
+++ b/sdbusplus/bus.hpp.in
@@ -36,8 +36,10 @@
{
void operator()(sd_bus* ptr) const
{
- sd_bus_unref(ptr);
+ deleter(ptr);
}
+
+ decltype(&sd_bus_flush_close_unref) deleter = sd_bus_flush_close_unref;
};
/* @brief Alias 'bus' to a unique_ptr type for auto-release. */
@@ -67,10 +69,18 @@
/** @brief Conversion constructor from 'busp_t'.
*
- * Takes ownership of the bus-pointer and releases it when done.
+ * Increments ref-count of the bus-pointer and releases it when done.
*/
explicit bus(busp_t b);
+ /** @brief Constructor for 'bus'.
+ *
+ * Takes ownership of the bus-pointer and releases it when done.
+ * This method will also cause the bus to be flushed and closed
+ * on destruction.
+ */
+ bus(busp_t b, std::false_type);
+
/** @brief Release ownership of the stored bus-pointer. */
busp_t release() { return _bus.release(); }
@@ -237,7 +247,24 @@
details::bus _bus;
};
-inline bus::bus(busp_t b) : _bus(b)
+inline bus::bus(busp_t b) : _bus(sd_bus_ref(b))
+{
+ _bus.get_deleter().deleter = sd_bus_unref;
+
+#if @WANT_TRANSACTION@
+ // Emitting object added causes a message to get the properties
+ // which can trigger a 'transaction' in the server bindings. If
+ // the bus isn't up far enough, this causes an assert deep in
+ // sd-bus code. Get the 'unique_name' to ensure the bus is up far
+ // enough to avoid the assert.
+ if (b != nullptr)
+ {
+ get_unique_name();
+ }
+#endif
+}
+
+inline bus::bus(busp_t b, std::false_type) : _bus(b)
{
#if @WANT_TRANSACTION@
// Emitting object added causes a message to get the properties
@@ -257,21 +284,21 @@
{
sd_bus* b = nullptr;
sd_bus_open(&b);
- return bus(b);
+ return bus(b, std::false_type());
}
inline bus new_user()
{
sd_bus* b = nullptr;
sd_bus_open_user(&b);
- return bus(b);
+ return bus(b, std::false_type());
}
inline bus new_system()
{
sd_bus* b = nullptr;
sd_bus_open_system(&b);
- return bus(b);
+ return bus(b, std::false_type());
}
} // namespace bus
@@ -284,7 +311,7 @@
{
sd_bus* b = nullptr;
b = sd_bus_message_get_bus(_msg.get());
- return bus::bus(sd_bus_ref(b));
+ return bus::bus(b);
}
} // namespace sdbusplus
diff --git a/sdbusplus/server/interface.hpp b/sdbusplus/server/interface.hpp
index db956e8..f3f6399 100644
--- a/sdbusplus/server/interface.hpp
+++ b/sdbusplus/server/interface.hpp
@@ -60,7 +60,7 @@
const char* interf,
const sdbusplus::vtable::vtable_t* vtable,
void* context) :
- _bus(sd_bus_ref(bus.get())), _path(path), _interf(interf),
+ _bus(bus.get()), _path(path), _interf(interf),
_slot(nullptr)
{
sd_bus_slot* slot = nullptr;
diff --git a/sdbusplus/server/object.hpp b/sdbusplus/server/object.hpp
index ddc7e17..6928164 100644
--- a/sdbusplus/server/object.hpp
+++ b/sdbusplus/server/object.hpp
@@ -89,7 +89,7 @@
const char* path,
bool deferSignal = false)
: details::compose<Args...>(bus, path),
- __sdbusplus_server_object_bus(sd_bus_ref(bus.get())),
+ __sdbusplus_server_object_bus(bus.get()),
__sdbusplus_server_object_path(path),
__sdbusplus_server_object_emitremoved(false)
{