openbmc / sdbusplus

C++ bindings for systemd dbus APIs
Apache License 2.0
101 stars 80 forks source link

(Proposition) Closing asio::connection, socket handling #63

Open vsabadazh opened 3 years ago

vsabadazh commented 3 years ago

Hello,

In our company, we have faced an issue which would lead our application to crash in certain circumstances (e.g. some of the signals keep coming after the connection was closed). We have modified the sdbusplus::asio::connection class so it handles the underlying socket in a more explicit way to close it, see below:

diff --git a/include/sdbusplus/asio/connection.hpp b/include/sdbusplus/asio/connection.hpp
index 15edd1b..b11a3d2 100644
--- a/include/sdbusplus/asio/connection.hpp
+++ b/include/sdbusplus/asio/connection.hpp
@@ -62,6 +62,12 @@ class connection : public sdbusplus::bus::bus
         socket.assign(get_fd());
         read_wait();
     }
+    connection(boost::asio::io_context& io, bus &&bus) :
+        sdbusplus::bus::bus(std::move(bus)), io_(io), socket(io_)
+    {
+        socket.assign(get_fd());
+        read_wait();
+    }
     ~connection()
     {
         // The FD will be closed by the socket object, so assign null to the
@@ -70,6 +76,13 @@ class connection : public sdbusplus::bus::bus
         socket.release();
     }

+    void close()
+    {
+        socket.cancel();
+        socket.release();
+        sdbusplus::bus::bus::close();
+    }
+
     /** @brief Perform an asynchronous send of a message, executing the handler
      *         upon return and return
      *
@@ -351,6 +364,10 @@ class connection : public sdbusplus::bus::bus
     void read_immediate()
     {
         boost::asio::post(io_, [&] {
+            if (!socket.is_open())
+            {
+                return;
+            }
             if (process_discard())
             {
                 read_immediate();

Applying it will improve stability of the library, which it did in our case.