Minor changes to ContainerConnection

[Bug/Feature]   ContainerConnection lifecycle is tied with a connection's lifecycle,
                OnNameLost callback added.
[Cause]         Be consistent, OnNameLost callback will be required to handle disconnections
[Solution]      N/A
[Verification]  Build, install, run tests

Change-Id: Ie43eda2a4774ef003bee9ed877b6caab041035ba
Signed-off-by: Lukasz Pawelczyk <l.pawelczyk@partner.samsung.com>

diff --git a/server/container-connection.cpp b/server/container-connection.cpp
index ed6bdd8..80d4be5 100644 (file)
--- a/server/container-connection.cpp
+++ b/server/container-connection.cpp
@@ -40,20 +40,10 @@ const unsigned int NAME_ACQUIRED_TIMEOUT = 5 * 1000;
 } // namespace
 
 
-ContainerConnection::ContainerConnection()
+ContainerConnection::ContainerConnection(const std::string& address, const OnNameLostCallback& callback)
     : mNameAcquired(false)
     , mNameLost(false)
 {
-}
-
-
-ContainerConnection::~ContainerConnection()
-{
-}
-
-
-void ContainerConnection::connect(const std::string& address)
-{
     if (address.empty()) {
         LOGW("The connection to the container is disabled");
         return;
@@ -61,15 +51,14 @@ void ContainerConnection::connect(const std::string& address)
 
     LOGT("Connecting to DBUS on " << address);
     mDbusConnection = dbus::DbusConnection::create(address);
-    LOGT("Setting DBUS name");
 
+    LOGT("Setting DBUS name");
     mDbusConnection->setName(api::BUS_NAME,
                              std::bind(&ContainerConnection::onNameAcquired, this),
                              std::bind(&ContainerConnection::onNameLost, this));
 
-    if (!waitForName(NAME_ACQUIRED_TIMEOUT)) {
+    if (!waitForNameAndSetCallback(NAME_ACQUIRED_TIMEOUT, callback)) {
         LOGE("Could not acquire dbus name: " << api::BUS_NAME);
-        disconnect();
         throw ContainerConnectionException("Could not acquire dbus name: " + api::BUS_NAME);
     }
 
@@ -87,18 +76,11 @@ void ContainerConnection::connect(const std::string& address)
     LOGD("Connected");
 }
 
-
-void ContainerConnection::disconnect()
+ContainerConnection::~ContainerConnection()
 {
-    LOGD("Disconnecting");
-    mDbusConnection.reset();
-
-    std::unique_lock<std::mutex> lock(mNameMutex);
-    mNameAcquired = false;
-    mNameLost = false;
 }
 
-bool ContainerConnection::waitForName(const unsigned int timeoutMs)
+bool ContainerConnection::waitForNameAndSetCallback(const unsigned int timeoutMs, const OnNameLostCallback& callback)
 {
     std::unique_lock<std::mutex> lock(mNameMutex);
     mNameCondition.wait_for(lock,
@@ -106,6 +88,10 @@ bool ContainerConnection::waitForName(const unsigned int timeoutMs)
                             [this] {
                                 return mNameAcquired || mNameLost;
                             });
+    if(mNameAcquired) {
+        mOnNameLostCallback = callback;
+    }
+
     return mNameAcquired;
 }
 
@@ -121,7 +107,10 @@ void ContainerConnection::onNameLost()
     std::unique_lock<std::mutex> lock(mNameMutex);
     mNameLost = true;
     mNameCondition.notify_one();
-    //TODO some callback?
+
+    if(mOnNameLostCallback) {
+        mOnNameLostCallback();
+    }
 }
 
 void ContainerConnection::setNotifyActiveContainerCallback(

diff --git a/server/container-connection.hpp b/server/container-connection.hpp
index f3c6f1d..4ddac0e 100644 (file)
--- a/server/container-connection.hpp
+++ b/server/container-connection.hpp
@@ -38,18 +38,10 @@ namespace security_containers {
 class ContainerConnection {
 
 public:
-    ContainerConnection();
-    ~ContainerConnection();
-
-    /**
-     * Connect to container.
-     */
-    void connect(const std::string& address);
+    typedef std::function<void()> OnNameLostCallback;
 
-    /**
-     * Disconnect from container.
-     */
-    void disconnect();
+    ContainerConnection(const std::string& address, const OnNameLostCallback& callback);
+    ~ContainerConnection();
 
     // ------------- API --------------
 
@@ -75,11 +67,12 @@ private:
     std::condition_variable mNameCondition;
     bool mNameAcquired;
     bool mNameLost;
+    OnNameLostCallback mOnNameLostCallback;
     NotifyActiveContainerCallback mNotifyActiveContainerCallback;
 
     void onNameAcquired();
     void onNameLost();
-    bool waitForName(const unsigned int timeoutMs);
+    bool waitForNameAndSetCallback(const unsigned int timeoutMs, const OnNameLostCallback& callback);
 
     void onMessageCall(const std::string& objectPath,
                        const std::string& interface,

diff --git a/server/container.cpp b/server/container.cpp
index 6474fb1..afa9ac8 100644 (file)
--- a/server/container.cpp
+++ b/server/container.cpp
@@ -67,7 +67,8 @@ void Container::start()
 {
     mConnectionTransport.reset(new ContainerConnectionTransport(mConfig.runMountPoint));
     mAdmin->start();
-    mConnection.connect(mConnectionTransport->acquireAddress());
+    mConnection.reset(new ContainerConnection(mConnectionTransport->acquireAddress(),
+                                              std::bind(&Container::onNameLostCallback, this)));
 
     // Send to the background only after we're connected,
     // otherwise it'd take ages.
@@ -77,7 +78,7 @@ void Container::start()
 
 void Container::stop()
 {
-    mConnection.disconnect();
+    mConnection.reset();
     mAdmin->stop();
     mConnectionTransport.reset();
 }
@@ -107,5 +108,10 @@ bool Container::isPaused()
     return mAdmin->isPaused();
 }
 
+void Container::onNameLostCallback()
+{
+    // TODO: try to reconnect
+}
+
 
 } // namespace security_containers

diff --git a/server/container.hpp b/server/container.hpp
index 179a3bb..89ef44e 100644 (file)
--- a/server/container.hpp
+++ b/server/container.hpp
@@ -100,7 +100,9 @@ private:
     ContainerConfig mConfig;
     std::unique_ptr<ContainerAdmin> mAdmin;
     std::unique_ptr<ContainerConnectionTransport> mConnectionTransport;
-    ContainerConnection mConnection;
+    std::unique_ptr<ContainerConnection> mConnection;
+
+    void onNameLostCallback();
 };
 
 

diff --git a/unit_tests/server/ut-container-connection.cpp b/unit_tests/server/ut-container-connection.cpp
index 95203b0..7721a3a 100644 (file)
--- a/unit_tests/server/ut-container-connection.cpp
+++ b/unit_tests/server/ut-container-connection.cpp
@@ -76,20 +76,12 @@ private:
 } // namespace
 
 
-BOOST_AUTO_TEST_CASE(ConstructorDestructorTest)
-{
-    BOOST_REQUIRE_NO_THROW(ContainerConnection());
-}
-
-BOOST_AUTO_TEST_CASE(ConnectTest)
+BOOST_AUTO_TEST_CASE(ConstructorDestructorConnectTest)
 {
     ScopedGlibLoop loop;
     ScopedDbusDaemon dbus;
 
-    ContainerConnection connection;
-
-    BOOST_REQUIRE_NO_THROW(connection.connect(dbus.acquireAddress()));
-    BOOST_REQUIRE_NO_THROW(connection.disconnect());
+    BOOST_REQUIRE_NO_THROW(ContainerConnection(dbus.acquireAddress(), nullptr));
 }
 
 BOOST_AUTO_TEST_CASE(NotifyActiveContainerApiTest)
@@ -98,16 +90,16 @@ BOOST_AUTO_TEST_CASE(NotifyActiveContainerApiTest)
     ScopedDbusDaemon dbus;
 
     Latch notifyCalled;
-    ContainerConnection connection;
+    std::unique_ptr<ContainerConnection> connection;
 
-    BOOST_REQUIRE_NO_THROW(connection.connect(dbus.acquireAddress()));
+    BOOST_REQUIRE_NO_THROW(connection.reset(new ContainerConnection(dbus.acquireAddress(), nullptr)));
 
     auto callback = [&](const std::string& application, const std::string& message) {
         if (application == "testapp" && message == "testmessage") {
             notifyCalled.set();
         }
     };
-    connection.setNotifyActiveContainerCallback(callback);
+    connection->setNotifyActiveContainerCallback(callback);
 
     DbusConnection::Pointer client = DbusConnection::create(dbus.acquireAddress());
     client->callMethod(api::BUS_NAME,
@@ -125,9 +117,9 @@ BOOST_AUTO_TEST_CASE(SignalNotificationApiTest)
     ScopedDbusDaemon dbus;
 
     Latch signalEmitted;
-    ContainerConnection connection;
+    std::unique_ptr<ContainerConnection> connection;
 
-    BOOST_REQUIRE_NO_THROW(connection.connect(dbus.acquireAddress()));
+    BOOST_REQUIRE_NO_THROW(connection.reset(new ContainerConnection(dbus.acquireAddress(), nullptr)));
 
     DbusConnection::Pointer client = DbusConnection::create(dbus.acquireAddress());
 
@@ -154,7 +146,7 @@ BOOST_AUTO_TEST_CASE(SignalNotificationApiTest)
     };
     client->signalSubscribe(handler, api::BUS_NAME);
 
-    connection.sendNotification("testcontainer", "testapp", "testmessage");
+    connection->sendNotification("testcontainer", "testapp", "testmessage");
 
     BOOST_CHECK(signalEmitted.wait(EVENT_TIMEOUT));
 }