Add unregister function to IObject and IProxy API

src/Message.cpp

@@ -658,10 +658,14 @@ AsyncMethodCall::AsyncMethodCall(MethodCall&& call) noexcept
 {
 }
 
-void AsyncMethodCall::send(void* callback, void* userData) const
+AsyncMethodCall::Slot AsyncMethodCall::send(void* callback, void* userData) const
 {
-    auto r = sdbus_->sd_bus_call_async(nullptr, nullptr, (sd_bus_message*)msg_, (sd_bus_message_handler_t)callback, userData, 0);
+    sd_bus_slot* slot;
+
+    auto r = sdbus_->sd_bus_call_async(nullptr, &slot, (sd_bus_message*)msg_, (sd_bus_message_handler_t)callback, userData, 0);
     SDBUS_THROW_ERROR_IF(r < 0, "Failed to call method asynchronously", -r);
+
+    return Slot{slot, [sdbus_ = sdbus_](void *slot){ sdbus_->sd_bus_slot_unref((sd_bus_slot*)slot); }};
 }
 
 void MethodReply::send() const

src/Object.cpp

@@ -120,6 +120,11 @@ void Object::finishRegistration()
     }
 }
 
+void Object::unregister()
+{
+    interfaces_.clear();
+}
+
 sdbus::Signal Object::createSignal(const std::string& interfaceName, const std::string& signalName)
 {
     return connection_.createSignal(objectPath_, interfaceName, signalName);

src/Object.h

@@ -73,6 +73,7 @@ namespace internal {
         void setInterfaceFlags(const std::string& interfaceName, Flags flags) override;
 
         void finishRegistration() override;
+        void unregister() override;
 
         sdbus::Signal createSignal(const std::string& interfaceName, const std::string& signalName) override;
         void emitSignal(const sdbus::Signal& message) override;

src/Proxy.cpp

@@ -28,6 +28,7 @@
 #include "sdbus-c++/Message.h"
 #include "sdbus-c++/IConnection.h"
 #include "sdbus-c++/Error.h"
+#include "ScopeGuard.h"
 #include <systemd/sd-bus.h>
 #include <cassert>
 #include <chrono>
@@ -40,8 +41,8 @@ Proxy::Proxy(sdbus::internal::IConnection& connection, std::string destination,
     , destination_(std::move(destination))
     , objectPath_(std::move(objectPath))
 {
-    // The connection is not ours only, it is managed by the client and we just reference it here,
-    // so we expect the client to manage the event loop upon this connection themselves.
+    // The connection is not ours only, it is owned and managed by the user and we just reference
+    // it here, so we expect the client to manage the event loop upon this connection themselves.
 }
 
 Proxy::Proxy( std::unique_ptr<sdbus::internal::IConnection>&& connection
@@ -51,8 +52,8 @@ Proxy::Proxy( std::unique_ptr<sdbus::internal::IConnection>&& connection
     , destination_(std::move(destination))
     , objectPath_(std::move(objectPath))
 {
-    // The connection is ours only, so we have to manage event loop upon this connection,
-    // so we get signals, async replies, and other messages from D-Bus.
+    // The connection is ours only, i.e. it's us who has to manage the event loop upon this connection,
+    // in order that we get and process signals, async call replies, and other messages from D-Bus.
     connection_->enterProcessingLoopAsync();
 }
 
@@ -74,10 +75,11 @@ MethodReply Proxy::callMethod(const MethodCall& message)
 void Proxy::callMethod(const AsyncMethodCall& message, async_reply_handler asyncReplyCallback)
 {
     auto callback = (void*)&Proxy::sdbus_async_reply_handler;
-    // Allocated userData gets deleted in the sdbus_async_reply_handler
-    auto userData = new AsyncReplyUserData{*this, std::move(asyncReplyCallback)};
+    auto callData = std::make_unique<AsyncCalls::CallData>(AsyncCalls::CallData{*this, std::move(asyncReplyCallback), {}});
 
-    message.send(callback, userData);
+    callData->slot = message.send(callback, callData.get());
+
+    pendingAsyncCalls_.addCall(callData->slot.get(), std::move(callData));
 }
 
 void Proxy::registerSignalHandler( const std::string& interfaceName
@@ -122,24 +124,32 @@ void Proxy::registerSignalHandlers(sdbus::internal::IConnection& connection)
     }
 }
 
+void Proxy::unregister()
+{
+    pendingAsyncCalls_.clear();
+    interfaces_.clear();
+}
+
 int Proxy::sdbus_async_reply_handler(sd_bus_message *sdbusMessage, void *userData, sd_bus_error */*retError*/)
 {
-    // We are assuming the ownership of the async reply handler pointer passed here
-    std::unique_ptr<AsyncReplyUserData> asyncReplyUserData{static_cast<AsyncReplyUserData*>(userData)};
-    assert(asyncReplyUserData != nullptr);
-    assert(asyncReplyUserData->callback);
+    auto* asyncCallData = static_cast<AsyncCalls::CallData*>(userData);
+    assert(asyncCallData != nullptr);
+    assert(asyncCallData->callback);
+    auto& proxy = asyncCallData->proxy;
 
-    MethodReply message{sdbusMessage, &asyncReplyUserData->proxy.connection_->getSdBusInterface()};
+    SCOPE_EXIT{ proxy.pendingAsyncCalls_.removeCall(asyncCallData->slot.get()); };
+
+    MethodReply message{sdbusMessage, &proxy.connection_->getSdBusInterface()};
 
     const auto* error = sd_bus_message_get_error(sdbusMessage);
     if (error == nullptr)
     {
-        asyncReplyUserData->callback(message, nullptr);
+        asyncCallData->callback(message, nullptr);
     }
     else
     {
         sdbus::Error exception(error->name, error->message);
-        asyncReplyUserData->callback(message, &exception);
+        asyncCallData->callback(message, &exception);
     }
 
     return 1;

src/Proxy.h

@@ -31,6 +31,8 @@
 #include <string>
 #include <memory>
 #include <map>
+#include <unordered_map>
+#include <mutex>
 
 // Forward declarations
 namespace sdbus { namespace internal {
@@ -60,14 +62,9 @@ namespace internal {
                                   , const std::string& signalName
                                   , signal_handler signalHandler ) override;
         void finishRegistration() override;
+        void unregister() override;
 
     private:
-        struct AsyncReplyUserData
-        {
-            Proxy& proxy;
-            async_reply_handler callback;
-        };
-
         void registerSignalHandlers(sdbus::internal::IConnection& connection);
         static int sdbus_async_reply_handler(sd_bus_message *sdbusMessage, void *userData, sd_bus_error *retError);
         static int sdbus_signal_callback(sd_bus_message *sdbusMessage, void *userData, sd_bus_error *retError);
@@ -91,6 +88,51 @@ namespace internal {
             std::map<SignalName, SignalData> signals_;
         };
         std::map<InterfaceName, InterfaceData> interfaces_;
+
+        // We need to keep track of pending async calls. When the proxy is being destructed, we must
+        // remove all slots of these pending calls, otherwise in case when the connection outlives
+        // the proxy, we might get async reply handlers invoked for pending async calls after the proxy
+        // has been destroyed, which is a free ticket into the realm of undefined behavior.
+        class AsyncCalls
+        {
+        public:
+            struct CallData
+            {
+                Proxy& proxy;
+                async_reply_handler callback;
+                AsyncMethodCall::Slot slot;
+            };
+
+            ~AsyncCalls()
+            {
+                clear();
+            }
+
+            bool addCall(void* slot, std::unique_ptr<CallData>&& asyncCallData)
+            {
+                std::lock_guard<std::mutex> lock(mutex_);
+                return calls_.emplace(slot, std::move(asyncCallData)).second;
+            }
+
+            bool removeCall(void* slot)
+            {
+                std::lock_guard<std::mutex> lock(mutex_);
+                return calls_.erase(slot) > 0;
+            }
+
+            void clear()
+            {
+                std::unique_lock<std::mutex> lock(mutex_);
+                auto asyncCallSlots = std::move(calls_);
+                // Perform releasing of sd-bus slots outside of the calls_ critical section which avoids
+                // double mutex dead lock when the async reply handler is invoked at the same time.
+                lock.unlock();
+            }
+
+        private:
+            std::unordered_map<void*, std::unique_ptr<CallData>> calls_;
+            std::mutex mutex_;
+        } pendingAsyncCalls_;
     };
 
 }}