Is destoring dbus object thread safe?

[Snarpix]

If I have understood correctly destroying object is currently not thread safe.

Let's look at example(modified, from https://github.com/Kistler-Group/sdbus-cpp/blob/master/docs/using-sdbus-c++.md):

class Concatenator : public sdbus::AdaptorInterfaces<org::sdbuscpp::Concatenator_adaptor /*, more adaptor classes if there are more interfaces*/>
{
public:
    Concatenator(sdbus::IConnection& connection, std::string objectPath)
        : AdaptorInterfaces(connection, std::move(objectPath))
    {
        registerAdaptor();
    }

    ~Concatenator()
    {
        unregisterAdaptor();
    }
   
   const std::string member_ = "12313123213";

protected:
    std::string concatenate(const std::vector<int32_t>& numbers, const std::string& separator) override
    {
            std::this_thread::sleep_for(300s);
           return member_;
    }
};

In this case calling unregisterAdaptor will stop new calls to concatenate method from happening, but if some call is already waiting on std::this_thread::sleep_for(300s); then when wait finishes it will access member_, which was already destroyed.

Is my reasoning correct?

[sangelovic]

Hello, no, it's not correct. The Object instance will not be destroyed as long as there are calls being served by that object. This is guaranteed by internal thread safety design of sdbus-c++. Thread safety chapter discusses this to some detail. It says it's completely safe to destroy Object and Proxy instances at any time. This of course counts with the fact that there may be operations pending on those instances at that very same time -- these operations are finished first and then the object is destroyed.

So in your case, the thread destructing the object would wait for at most 300 seconds until the event loop thread serving the object finishes serving the call.

[Snarpix]

I'm trying to find where this logic is implemented, because I want to be aware of potential deadlocks, for example on delete this.
Calling unregisterAdaptor() destroys interfaces_ and objectManagerSlot_.
objectManagerSlot_ seems to be unrelated to my problem.

interfaces_ contains slot that is filled from Connection::addObjectVTable.
slot contains destructor with sd_bus_slot_unref

return {slot, [this](void *slot){ iface_->sd_bus_slot_unref((sd_bus_slot*)slot); }};

So sd_bus_slot_unref must block in order to wait for method return until destruction.
Looking at https://github.com/systemd/systemd/blob/main/src/libsystemd/sd-bus/bus-slot.c#LL179C1-L179C1
I can't see anything that could potentially block until method call returns.

Could you please give me a hint where I can find function that will block destruction until the dbus method returns?

[Snarpix]

Okay, I have found the mutex, it's inside sd_bus_slot* SdBus::sd_bus_slot_unref(sd_bus_slot *slot) function.
And when callback is processing int SdBus::sd_bus_process(sd_bus *bus, sd_bus_message **r) will lock the same mutex.
Since mutex is recursive everything should work fine

[sangelovic]

Yes, it's exactly that mutex that ensures serialized access to the underlying systemd objects.