[NetworkBearerSelection] Fix for potential memory leaks.

[Verification] Code compiles. TCT pass rate 100%.

Change-Id: Idba09da6bcf7a69e959881bb35ea963e9f2b2bdb
Signed-off-by: Tomasz Marciniak <t.marciniak@samsung.com>

diff --git a/src/networkbearerselection/networkbearerselection_manager.cc b/src/networkbearerselection/networkbearerselection_manager.cc
index b6675ff410dcc0d7e7fae099c6de88bb719acbcd..1abbbf3aa529bafa2e1ca3c77c6d87a0f71d7fd8 100644 (file)
--- a/src/networkbearerselection/networkbearerselection_manager.cc
+++ b/src/networkbearerselection/networkbearerselection_manager.cc
@@ -16,6 +16,7 @@
 
 #include "networkbearerselection_manager.h"
 #include "common/logger.h"
+#include "common/scope_exit.h"
 
 #include <netdb.h>
 #include <arpa/inet.h>
@@ -45,15 +46,15 @@ struct NetworkBearerSelectionReleaseEvent {
 void NetworkBearerSelectionManager::AddListener(
     NetworkBearerSelectionListener* listener) {
   LoggerD("Enter");
-  std::lock_guard<std::mutex> lock(m_mutex);
-  m_listeners.push_back(listener);
+  std::lock_guard<std::mutex> lock(m_mutex_);
+  m_listeners_.push_back(listener);
 }
 
 void NetworkBearerSelectionManager::RemoveListener(
     NetworkBearerSelectionListener* listener) {
   LoggerD("Enter");
-  std::lock_guard<std::mutex> lock(m_mutex);
-  m_listeners.remove(listener);
+  std::lock_guard<std::mutex> lock(m_mutex_);
+  m_listeners_.remove(listener);
 }
 
 NetworkBearerSelectionManager* NetworkBearerSelectionManager::GetInstance() {
@@ -63,33 +64,42 @@ NetworkBearerSelectionManager* NetworkBearerSelectionManager::GetInstance() {
 }
 
 NetworkBearerSelectionManager::NetworkBearerSelectionManager()
-    : m_connectionHandle(nullptr),
-      m_profileHandle(nullptr),
-      m_connectionState(ConnectionState::Unknown),
-      m_isConnectionOpen(false) {
+    : m_connection_handle_(nullptr),
+      m_profile_handle_(nullptr),
+      m_connection_state_(ConnectionState::Unknown),
+      m_is_connection_open_(false) {
   LoggerD("Enter");
-  int ret = connection_create(&m_connectionHandle);
+  int ret = connection_create(&m_connection_handle_);
 
   if (CONNECTION_ERROR_NONE == ret) {
     LoggerD("Client registration success");
   } else {
     LoggerE("Client registration failed");
-    m_connectionHandle = nullptr;
+    m_connection_handle_ = nullptr;
   }
 }
 
 NetworkBearerSelectionManager::~NetworkBearerSelectionManager() {
   LoggerD("Enter");
-  if (m_connectionHandle != nullptr) {
+  if (m_connection_handle_ != nullptr) {
     LoggerD("Client deregistration success");
-    if (m_profileHandle) {
-      connection_profile_destroy(m_profileHandle);
-      m_profileHandle = nullptr;
-    }
-    connection_destroy(m_connectionHandle);
+    destroyProfileHandle();
+    connection_destroy(m_connection_handle_);
   } else {
     LoggerE("Client deregistration failed");
   }
+
+  {
+    std::lock_guard<std::mutex> lock(m_request_mutex_);
+    LoggerD("Delete %d request object(s)", m_request_events_.size());
+    m_request_events_.clear();
+  }
+
+  {
+    std::lock_guard<std::mutex> lock(m_release_mutex_);
+    LoggerD("Delete %d release object(s)", m_release_events_.size());
+    m_release_events_.clear();
+  }
 }
 
 void NetworkBearerSelectionManager::connection_state_changed_callback(
@@ -102,13 +112,17 @@ void NetworkBearerSelectionManager::connection_state_changed_callback(
       LoggerD("association state");
       return;
     }
-    NetworkBearerSelectionRequestEvent* event =
-        static_cast<NetworkBearerSelectionRequestEvent*>(user_data);
+
+    RequestEventPtr event = NetworkBearerSelectionManager::GetInstance()->getRequestEvent(
+        static_cast<NetworkBearerSelectionRequestEvent*>(user_data));
+    if (!event) {
+      LoggerD("Event is not found.");
+      return;
+    }
 
     if (state == CONNECTION_PROFILE_STATE_DISCONNECTED) {
       std::string domain_name = event->domain_name;
       NetworkBearerSelectionManager::GetInstance()->deregistStateChangeListener(domain_name);
-      delete event;
 
       NetworkBearerSelectionManager::GetInstance()->makeDisconnectCallback(domain_name);
     }
@@ -124,10 +138,14 @@ void NetworkBearerSelectionManager::connection_profile_opened_callback(
     return;
   }
 
-  NetworkBearerSelectionRequestEvent* event =
-      static_cast<NetworkBearerSelectionRequestEvent*>(user_data);
+  RequestEventPtr event = NetworkBearerSelectionManager::GetInstance()->getRequestEvent(
+      static_cast<NetworkBearerSelectionRequestEvent*>(user_data));
+  if (!event) {
+    LoggerD("Event is not found.");
+    return;
+  }
+
   std::string domain_name = event->domain_name;
-  delete event;
 
   if (result == CONNECTION_ERROR_NONE) {
     LoggerD("Connection open Succeeded");
@@ -151,11 +169,15 @@ void NetworkBearerSelectionManager::connection_closed_callback(
     return;
   }
 
-  NetworkBearerSelectionReleaseEvent* event =
-      static_cast<NetworkBearerSelectionReleaseEvent*>(user_data);
+  ReleaseEventPtr event = NetworkBearerSelectionManager::GetInstance()->getReleaseEvent(
+      static_cast<NetworkBearerSelectionReleaseEvent*>(user_data));
+  if (!event) {
+    LoggerD("Event is not found.");
+    return;
+  }
+
   std::string domain_name = event->domain_name;
   ReleaseReplyCallback callback = event->callback;
-  delete event;
 
   if (result == CONNECTION_ERROR_NONE) {
     LoggerD("Connection close Succeeded");
@@ -169,24 +191,15 @@ void NetworkBearerSelectionManager::connection_closed_callback(
   }
 }
 
-void NetworkBearerSelectionManager::connection_closed_callback2(
-    connection_error_e result,
-    void* user_data) {
-  LoggerD("enter");
-  if (result == CONNECTION_ERROR_NONE) {
-    LoggerD("Connection close Succeeded");
-  }
-}
-
 void NetworkBearerSelectionManager::requestRouteToHost(
     const std::string& domain_name) {
   LoggerD("NetworkBearerSelectionManager::requestRouteToHost");
   connection_profile_h profileHandle;
 
-  if (m_connectionState == ConnectionState::Connected) {
+  if (m_connection_state_ == ConnectionState::Connected) {
     LoggerD("connection is already opened.");
-    for (std::list<std::string>::iterator it = m_domainNames.begin();
-         it != m_domainNames.end();
+    for (std::list<std::string>::iterator it = m_domain_names_.begin();
+         it != m_domain_names_.end();
          it++) {
       if (*it == domain_name) {
         LoggerD("Same domain name is exist in list.");
@@ -196,15 +209,12 @@ void NetworkBearerSelectionManager::requestRouteToHost(
     }
   }
 
-  if (m_profileHandle) {
-    connection_profile_destroy(m_profileHandle);
-    m_profileHandle = nullptr;
-  }
+  destroyProfileHandle();
 
   if (connection_get_default_cellular_service_profile(
-          m_connectionHandle,
+          m_connection_handle_,
           CONNECTION_CELLULAR_SERVICE_TYPE_INTERNET,
-          &m_profileHandle) != CONNECTION_ERROR_NONE) {
+          &m_profile_handle_) != CONNECTION_ERROR_NONE) {
     LoggerE("Fail to get profile handle");
     makeErrorCallback(domain_name, kPlatformError);
     return;
@@ -213,7 +223,7 @@ void NetworkBearerSelectionManager::requestRouteToHost(
   char* defaultProfileName_c = nullptr;
   std::string defaultProfileName;
 
-  connection_profile_get_name(m_profileHandle, &defaultProfileName_c);
+  connection_profile_get_name(m_profile_handle_, &defaultProfileName_c);
   if (defaultProfileName_c == nullptr) {
     LoggerE("default profile is not exist.");
     makeErrorCallback(domain_name, kPlatformError);
@@ -223,7 +233,7 @@ void NetworkBearerSelectionManager::requestRouteToHost(
   free(defaultProfileName_c);
   defaultProfileName_c = nullptr;
 
-  if (connection_get_current_profile(m_connectionHandle, &profileHandle) !=
+  if (connection_get_current_profile(m_connection_handle_, &profileHandle) !=
       CONNECTION_ERROR_NONE) {
     LoggerE("Fail to get current profile handle");
     makeErrorCallback(domain_name, kPlatformError);
@@ -249,17 +259,19 @@ void NetworkBearerSelectionManager::requestRouteToHost(
   currentProfileName_c = nullptr;
 
   if (defaultProfileName != currentProfileName) {
-    NetworkBearerSelectionRequestEvent* event =
-        new NetworkBearerSelectionRequestEvent(domain_name);
-    if (connection_open_profile(m_connectionHandle,
-                                m_profileHandle,
+    RequestEventPtr event(new NetworkBearerSelectionRequestEvent(domain_name));
+
+    if (connection_open_profile(m_connection_handle_,
+                                m_profile_handle_,
                                 connection_profile_opened_callback,
-                                event) != CONNECTION_ERROR_NONE) {
+                                event.get()) != CONNECTION_ERROR_NONE) {
       LoggerE("Connection open Failed");
-      delete event;
       makeErrorCallback(domain_name, kPlatformError);
     } else {
-      m_isConnectionOpen = true;
+      m_is_connection_open_ = true;
+
+      std::lock_guard<std::mutex> lock(m_request_mutex_);
+      m_request_events_.push_back(event);
     }
   } else {
     registStateChangeListener(domain_name);
@@ -270,39 +282,38 @@ common::PlatformResult NetworkBearerSelectionManager::releaseRouteToHost(
     const std::string& domain_name, const ReleaseReplyCallback& reply_cb) {
   LoggerD("enter");
 
-  for (const auto& name : m_domainNames) {
+  for (const auto& name : m_domain_names_) {
     if (name == domain_name) {
       LoggerD("Same domain name is exist in list.");
-      m_domainNames.remove(domain_name);
-      LoggerD("list size : %i", m_domainNames.size());
-      if (m_domainNames.size() == 0) {
-        if (!m_profileHandle) {
+      m_domain_names_.remove(domain_name);
+      LoggerD("list size : %i", m_domain_names_.size());
+      if (m_domain_names_.size() == 0) {
+        if (!m_profile_handle_) {
           return common::PlatformResult(common::ErrorCode::UNKNOWN_ERR, "Already in use");
         }
 
-        if (connection_profile_unset_state_changed_cb(m_profileHandle) !=
+        if (connection_profile_unset_state_changed_cb(m_profile_handle_) !=
             CONNECTION_ERROR_NONE) {
           LoggerE("unset callback is failed");
-          if (m_profileHandle) {
-            connection_profile_destroy(m_profileHandle);
-            m_profileHandle = NULL;
-          }
+          destroyProfileHandle();
           return common::PlatformResult(common::ErrorCode::NO_ERROR);
         }
 
-        if (m_isConnectionOpen) {
-          NetworkBearerSelectionReleaseEvent* event =
-              new NetworkBearerSelectionReleaseEvent(domain_name, reply_cb);
-          if (connection_close_profile(m_connectionHandle,
-                                       m_profileHandle,
+        if (m_is_connection_open_) {
+          ReleaseEventPtr event(new NetworkBearerSelectionReleaseEvent(domain_name, reply_cb));
+
+          if (connection_close_profile(m_connection_handle_,
+                                       m_profile_handle_,
                                        connection_closed_callback,
-                                       event) != CONNECTION_ERROR_NONE) {
+                                       event.get()) != CONNECTION_ERROR_NONE) {
             LoggerE("connection close failed");
-            delete event;
             reply_cb(false);
           } else {
-            m_isConnectionOpen = false;
+            m_is_connection_open_ = false;
             deregistStateChangeListener(domain_name);
+
+            std::lock_guard<std::mutex> lock(m_release_mutex_);
+            m_release_events_.push_back(event);
           }
         } else {
           reply_cb(true);
@@ -323,13 +334,17 @@ void NetworkBearerSelectionManager::registStateChangeListener(
   std::unique_ptr<char, void(*)(void*)> host_addr_ptr(nullptr, &std::free);
   struct addrinfo* servinfo = nullptr;
 
+  SCOPE_EXIT {
+    if (interfaceName) {
+      free(interfaceName);
+    }
+    freeaddrinfo(servinfo);
+  };
+
   if (connection_profile_get_network_interface_name(
-          m_profileHandle, &interfaceName) != CONNECTION_ERROR_NONE) {
+          m_profile_handle_, &interfaceName) != CONNECTION_ERROR_NONE) {
     LoggerE("Fail to get interface name!");
-    if (m_profileHandle) {
-      connection_profile_destroy(m_profileHandle);
-      m_profileHandle = nullptr;
-    }
+    destroyProfileHandle();
     makeErrorCallback(domain_name, kPlatformError);
   } else {
     LoggerD("Interface name : %s", interfaceName);
@@ -340,10 +355,7 @@ void NetworkBearerSelectionManager::registStateChangeListener(
   int ret_val = getaddrinfo(domain_name.c_str() , nullptr , nullptr , &servinfo);
   if (0 != ret_val) {
     LoggerE("Error while calling getaddrinfo(): %s", gai_strerror(ret_val));
-    if (m_profileHandle) {
-      connection_profile_destroy(m_profileHandle);
-      m_profileHandle = nullptr;
-    }
+    destroyProfileHandle();
     makeErrorCallback(domain_name, kPlatformError);
     return;
   } else {
@@ -360,61 +372,54 @@ void NetworkBearerSelectionManager::registStateChangeListener(
     }
     if (nullptr == inet_ntop(servinfo->ai_family, addr, hostAddr, servinfo->ai_addrlen)) {
       LoggerE("Error while calling inet_ntop()");
-      if (m_profileHandle) {
-        connection_profile_destroy(m_profileHandle);
-        m_profileHandle = nullptr;
-      }
+      destroyProfileHandle();
       makeErrorCallback(domain_name, kPlatformError);
-      freeaddrinfo(servinfo);
       return;
     }
     LoggerD("hostAddr : %s", hostAddr);
-
-    freeaddrinfo(servinfo);
   }
 
-  NetworkBearerSelectionRequestEvent* event =
-      new NetworkBearerSelectionRequestEvent(domain_name);
-  if (connection_profile_set_state_changed_cb(m_profileHandle,
+  RequestEventPtr event(new NetworkBearerSelectionRequestEvent(domain_name));
+
+  if (connection_profile_set_state_changed_cb(m_profile_handle_,
                                               connection_state_changed_callback,
-                                              event) != CONNECTION_ERROR_NONE) {
+                                              event.get()) != CONNECTION_ERROR_NONE) {
     LoggerE("Callback register is failed.");
-    if (m_profileHandle) {
-      connection_profile_destroy(m_profileHandle);
-      m_profileHandle = nullptr;
-    }
-    delete event;
+    destroyProfileHandle();
   } else {
-    if (connection_add_route(m_connectionHandle, interfaceName, hostAddr) !=
+    if (connection_add_route(m_connection_handle_, interfaceName, hostAddr) !=
         CONNECTION_ERROR_NONE) {
       LoggerE("add route is failed.");
-      connection_profile_unset_state_changed_cb(m_profileHandle);
+      connection_profile_unset_state_changed_cb(m_profile_handle_);
       makeErrorCallback(domain_name, kPlatformError);
     } else {
       LoggerD("add route is successed.");
-      m_domainNames.push_back(domain_name);
+      m_domain_names_.push_back(domain_name);
       makeSuccessCallback(domain_name);
     }
+
+    std::lock_guard<std::mutex> lock(m_request_mutex_);
+    m_request_events_.push_back(event);
   }
 }
 
 void NetworkBearerSelectionManager::deregistStateChangeListener(
     const std::string& domain_name) {
   LoggerD("enter");
-  if (m_profileHandle) {
-    connection_profile_unset_state_changed_cb(m_profileHandle);
-    connection_profile_destroy(m_profileHandle);
-    m_profileHandle = NULL;
+  if (m_profile_handle_) {
+    connection_profile_unset_state_changed_cb(m_profile_handle_);
+    connection_profile_destroy(m_profile_handle_);
+    m_profile_handle_ = nullptr;
   }
-  m_domainNames.remove(domain_name);
-  m_connectionState = ConnectionState::Disconnected;
+  m_domain_names_.remove(domain_name);
+  m_connection_state_ = ConnectionState::Disconnected;
 }
 
 void NetworkBearerSelectionManager::makeSuccessCallback(
     const std::string& domain_name) {
   LoggerD("enter");
-  std::lock_guard<std::mutex> lock(m_mutex);
-  for (NetworkBearerSelectionListener* listener : m_listeners)
+  std::lock_guard<std::mutex> lock(m_mutex_);
+  for (NetworkBearerSelectionListener* listener : m_listeners_)
     listener->onNBSSuccess(domain_name);
 }
 
@@ -430,18 +435,54 @@ void NetworkBearerSelectionManager::makeErrorCallback(
     const std::string& domain_name,
     const std::string& info) {
   LoggerD("enter");
-  std::lock_guard<std::mutex> lock(m_mutex);
-  for (NetworkBearerSelectionListener* listener : m_listeners)
+  std::lock_guard<std::mutex> lock(m_mutex_);
+  for (NetworkBearerSelectionListener* listener : m_listeners_)
     listener->onNBSError(domain_name, info);
 }
 
 void NetworkBearerSelectionManager::makeDisconnectCallback(
     const std::string& domain_name) {
   LoggerD("enter");
-  std::lock_guard<std::mutex> lock(m_mutex);
-  for (NetworkBearerSelectionListener* listener : m_listeners)
+  std::lock_guard<std::mutex> lock(m_mutex_);
+  for (NetworkBearerSelectionListener* listener : m_listeners_)
     listener->onNBSDisconnect(domain_name);
 }
 
+void NetworkBearerSelectionManager::destroyProfileHandle() {
+  LoggerD("enter");
+  if (m_profile_handle_) {
+    connection_profile_destroy(m_profile_handle_);
+    m_profile_handle_ = nullptr;
+  }
+}
+
+RequestEventPtr NetworkBearerSelectionManager::getRequestEvent(NetworkBearerSelectionRequestEvent* event) {
+  LoggerD("enter");
+  std::lock_guard<std::mutex> lock(m_request_mutex_);
+  for (auto it = m_request_events_.begin(); it != m_request_events_.end(); it++) {
+    if (it->get() == event) {
+      LoggerD("Found object [%p]", it->get());
+      RequestEventPtr ev = *it;
+      m_request_events_.erase(it);
+      return ev;
+    }
+  }
+  return nullptr;
+}
+
+ReleaseEventPtr NetworkBearerSelectionManager::getReleaseEvent(NetworkBearerSelectionReleaseEvent* event) {
+  LoggerD("enter");
+  std::lock_guard<std::mutex> lock(m_release_mutex_);
+  for (auto it = m_release_events_.begin(); it != m_release_events_.end(); it++) {
+    if (it->get() == event) {
+      LoggerD("Found object [%p]", it->get());
+      ReleaseEventPtr ev = *it;
+      m_release_events_.erase(it);
+      return ev;
+    }
+  }
+  return nullptr;
+}
+
 }  // namespace networkbearerselection
 }  // namespace extension

diff --git a/src/networkbearerselection/networkbearerselection_manager.h b/src/networkbearerselection/networkbearerselection_manager.h
index b5e1c1ebad1b1ab73f6d50344aedf3a58df3eeae..baddec504cd7b62ae12c2e99c5359eb3bf0b5153 100644 (file)
--- a/src/networkbearerselection/networkbearerselection_manager.h
+++ b/src/networkbearerselection/networkbearerselection_manager.h
@@ -19,7 +19,9 @@
 
 #include <string>
 #include <list>
+#include <vector>
 #include <mutex>
+#include <memory>
 #include <functional>
 #include <device/callback.h>
 #include <net_connection.h>
@@ -41,6 +43,12 @@ enum class NetworkType {
   Unknown
 };
 
+struct NetworkBearerSelectionRequestEvent;
+struct NetworkBearerSelectionReleaseEvent;
+
+typedef std::shared_ptr<NetworkBearerSelectionRequestEvent> RequestEventPtr;
+typedef std::shared_ptr<NetworkBearerSelectionReleaseEvent> ReleaseEventPtr;
+
 class NetworkBearerSelectionListener {
  public:
   virtual void onNBSSuccess(const std::string& domain_name) = 0;
@@ -73,8 +81,6 @@ class NetworkBearerSelectionManager {
                                                  void* user_data);
   static void connection_closed_callback(connection_error_e result,
                                          void* user_data);
-  static void connection_closed_callback2(connection_error_e result,
-                                          void* user_data);
 
   void registStateChangeListener(const std::string& domain_name);
   void deregistStateChangeListener(const std::string& domain_name);
@@ -84,18 +90,26 @@ class NetworkBearerSelectionManager {
   void makeErrorCallback(const std::string& domain_name,
                          const std::string& info);
   void makeDisconnectCallback(const std::string& domain_name);
+  void destroyProfileHandle();
+  RequestEventPtr getRequestEvent(NetworkBearerSelectionRequestEvent* event);
+  ReleaseEventPtr getReleaseEvent(NetworkBearerSelectionReleaseEvent* event);
 
   NetworkBearerSelectionManager();
   ~NetworkBearerSelectionManager();
 
-  std::list<NetworkBearerSelectionListener*> m_listeners;
+  std::list<NetworkBearerSelectionListener*> m_listeners_;
+
+  connection_h m_connection_handle_;
+  connection_profile_h m_profile_handle_;
+  std::list<std::string> m_domain_names_;
+  std::vector<RequestEventPtr> m_request_events_;
+  std::vector<ReleaseEventPtr> m_release_events_;
+  ConnectionState m_connection_state_;
+  bool m_is_connection_open_;
 
-  connection_h m_connectionHandle;
-  connection_profile_h m_profileHandle;
-  std::list<std::string> m_domainNames;
-  ConnectionState m_connectionState;
-  bool m_isConnectionOpen;
-  std::mutex m_mutex;
+  std::mutex m_mutex_;
+  std::mutex m_request_mutex_;
+  std::mutex m_release_mutex_;
 };
 
 }  // namespace networkbearerselection