[dbus] fix dbus reconnection (#1198)

Found that `dbus_bus_request_name` may fail on Raspberry Pi.

This commit fixes the issue by re-connecting DBus in such case.

2 files changed, 53 insertions, 36 deletions

diff --git a/src/dbus/server/dbus_agent.cpp b/src/dbus/server/dbus_agent.cpp
index d3e4a676..905f3a30 100644
--- a/src/dbus/server/dbus_agent.cpp
+++ b/src/dbus/server/dbus_agent.cpp
@@ -31,6 +31,7 @@
 #include "dbus/server/dbus_agent.hpp"
 
 #include <chrono>
+#include <thread>
 #include <unistd.h>
 
 #include "common/logging.hpp"
@@ -39,7 +40,8 @@
 namespace otbr {
 namespace DBus {
 
-const struct timeval DBusAgent::kPollTimeout = {0, 0};
+const struct timeval           DBusAgent::kPollTimeout = {0, 0};
+constexpr std::chrono::seconds DBusAgent::kDBusWaitAllowance;
 
 DBusAgent::DBusAgent(otbr::Ncp::ControllerOpenThread &aNcp)
     : mInterfaceName(aNcp.GetInterfaceName())
@@ -49,47 +51,57 @@ DBusAgent::DBusAgent(otbr::Ncp::ControllerOpenThread &aNcp)
 
 void DBusAgent::Init(void)
 {
-    DBusError   dbusError;
-    otbrError   error = OTBR_ERROR_NONE;
-    int         requestReply;
-    std::string serverName = OTBR_DBUS_SERVER_PREFIX + mInterfaceName;
+    otbrError error = OTBR_ERROR_NONE;
 
-    dbus_error_init(&dbusError);
-    DBusConnection *conn                = nullptr;
-    auto            connection_deadline = std::chrono::steady_clock::now() + std::chrono::seconds(30);
+    auto connection_deadline = Clock::now() + kDBusWaitAllowance;
 
-    while (true)
+    while ((mConnection = PrepareDBusConnection()) == nullptr && Clock::now() < connection_deadline)
     {
-        conn = dbus_bus_get(DBUS_BUS_SYSTEM, &dbusError);
-        if (conn != nullptr || std::chrono::steady_clock::now() > connection_deadline)
-        {
-            break;
-        }
-        otbrLogWarning("Failed to get DBus connection: %s: %s, will retry after 3 seconds", dbusError.name,
-                       dbusError.message);
-        dbus_error_free(&dbusError);
-        sleep(3);
+        otbrLogWarning("Failed to setup DBus connection, will retry after 1 second");
+        std::this_thread::sleep_for(std::chrono::seconds(1));
     }
-    mConnection = std::unique_ptr<DBusConnection, std::function<void(DBusConnection *)>>(
-        conn, [](DBusConnection *aConnection) { dbus_connection_unref(aConnection); });
+
     VerifyOrDie(mConnection != nullptr, "Failed to get DBus connection");
-    dbus_bus_register(mConnection.get(), &dbusError);
+
+    mThreadObject = std::unique_ptr<DBusThreadObject>(new DBusThreadObject(mConnection.get(), mInterfaceName, &mNcp));
+    error         = mThreadObject->Init();
+    VerifyOrDie(error == OTBR_ERROR_NONE, "Failed to initialize DBus Agent");
+}
+
+DBusAgent::UniqueDBusConnection DBusAgent::PrepareDBusConnection(void)
+{
+    DBusError            dbusError;
+    DBusConnection *     conn = nullptr;
+    UniqueDBusConnection uniqueConn;
+    int                  requestReply;
+    std::string          serverName = OTBR_DBUS_SERVER_PREFIX + mInterfaceName;
+
+    dbus_error_init(&dbusError);
+
+    conn = dbus_bus_get(DBUS_BUS_SYSTEM, &dbusError);
+
+    uniqueConn = UniqueDBusConnection(conn, [](DBusConnection *aConnection) { dbus_connection_unref(aConnection); });
+
+    VerifyOrExit(uniqueConn != nullptr,
+                 otbrLogWarning("Failed to get DBus connection: %s: %s", dbusError.name, dbusError.message));
+    dbus_bus_register(uniqueConn.get(), &dbusError);
+
     requestReply =
-        dbus_bus_request_name(mConnection.get(), serverName.c_str(), DBUS_NAME_FLAG_REPLACE_EXISTING, &dbusError);
+        dbus_bus_request_name(uniqueConn.get(), serverName.c_str(), DBUS_NAME_FLAG_REPLACE_EXISTING, &dbusError);
     VerifyOrExit(requestReply == DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER ||
                      requestReply == DBUS_REQUEST_NAME_REPLY_ALREADY_OWNER,
-                 error = OTBR_ERROR_DBUS);
+                 {
+                     otbrLogWarning("Failed to request DBus name: %s: %s", dbusError.name, dbusError.message);
+                     uniqueConn = nullptr;
+                 });
     VerifyOrExit(
-        dbus_connection_set_watch_functions(mConnection.get(), AddDBusWatch, RemoveDBusWatch, nullptr, this, nullptr));
-    mThreadObject = std::unique_ptr<DBusThreadObject>(new DBusThreadObject(mConnection.get(), mInterfaceName, &mNcp));
-    error         = mThreadObject->Init();
+        dbus_connection_set_watch_functions(uniqueConn.get(), AddDBusWatch, RemoveDBusWatch, nullptr, this, nullptr),
+        uniqueConn = nullptr);
+
 exit:
-    if (error == OTBR_ERROR_DBUS)
-    {
-        otbrLogErr("DBus error %s: %s", dbusError.name, dbusError.message);
-    }
-    VerifyOrDie(error == OTBR_ERROR_NONE, "Failed to initialize DBus Agent");
     dbus_error_free(&dbusError);
+
+    return uniqueConn;
 }
 
 dbus_bool_t DBusAgent::AddDBusWatch(struct DBusWatch *aWatch, void *aContext)
diff --git a/src/dbus/server/dbus_agent.hpp b/src/dbus/server/dbus_agent.hpp
index 394852bf..ae5ed126 100644
--- a/src/dbus/server/dbus_agent.hpp
+++ b/src/dbus/server/dbus_agent.hpp
@@ -72,16 +72,21 @@ public:
     void Process(const MainloopContext &aMainloop) override;
 
 private:
-    static dbus_bool_t AddDBusWatch(struct DBusWatch *aWatch, void *aContext);
-    static void        RemoveDBusWatch(struct DBusWatch *aWatch, void *aContext);
+    using Clock                                              = std::chrono::steady_clock;
+    constexpr static std::chrono::seconds kDBusWaitAllowance = std::chrono::seconds(30);
+
+    using UniqueDBusConnection = std::unique_ptr<DBusConnection, std::function<void(DBusConnection *)>>;
+
+    static dbus_bool_t   AddDBusWatch(struct DBusWatch *aWatch, void *aContext);
+    static void          RemoveDBusWatch(struct DBusWatch *aWatch, void *aContext);
+    UniqueDBusConnection PrepareDBusConnection(void);
 
     static const struct timeval kPollTimeout;
 
     std::string                       mInterfaceName;
     std::unique_ptr<DBusThreadObject> mThreadObject;
-    using UniqueDBusConnection = std::unique_ptr<DBusConnection, std::function<void(DBusConnection *)>>;
-    UniqueDBusConnection             mConnection;
-    otbr::Ncp::ControllerOpenThread &mNcp;
+    UniqueDBusConnection              mConnection;
+    otbr::Ncp::ControllerOpenThread & mNcp;
 
     /**
      * This map is used to track DBusWatch-es.