Fix memory leak 57/296657/3
authorHwankyu Jhun <h.jhun@samsung.com>
Thu, 3 Aug 2023 00:25:19 +0000 (09:25 +0900)
committerHwankyu Jhun <h.jhun@samsung.com>
Thu, 3 Aug 2023 05:22:53 +0000 (14:22 +0900)
If the callee application does not use tizen main loop function,
the ResultCb instance is not released.
To release the allocated memory properly, this patch adds a new method to
the ClientChannel. When calling the OnTimedOut, amd releases the ResultCb.

Change-Id: Ia29e7ba632296c56ae95d9fd5febc14ecfcd9e22
Signed-off-by: Hwankyu Jhun <h.jhun@samsung.com>
src/lib/api/amd_api_app_request_broker.cc

index 32fd513..b0812dc 100644 (file)
@@ -23,6 +23,7 @@
 #include <gio/gio.h>
 #include <glib.h>
 
+#include <list>
 #include <memory>
 #include <queue>
 #include <string>
@@ -41,8 +42,16 @@ constexpr const char PATH_AUL_APPS[] = "/run/aul/apps";
 
 class ResultCb {
  public:
-  ResultCb(unsigned int timeout, amd_app_request_broker_result_cb cb,
-      void* user_data) : cb_(cb), user_data_(user_data) {
+  class IEvent {
+   public:
+    virtual ~IEvent() = default;
+    virtual void OnTimedOut(ResultCb* cb) = 0;
+  };
+
+  ResultCb(IEvent* listener, unsigned int timeout,
+      amd_app_request_broker_result_cb cb,
+      void* user_data)
+      : listener_(listener), cb_(cb), user_data_(user_data) {
     timer_ = g_timeout_add(timeout, OnTimedOut, this);
   }
 
@@ -61,20 +70,27 @@ class ResultCb {
  private:
   static gboolean OnTimedOut(gpointer user_data) {
     auto* cb = static_cast<ResultCb*>(user_data);
-    cb->Invoke(-ETIMEDOUT);
     cb->timer_ = 0;
+
+    auto* listener = cb->listener_;
+    if (listener != nullptr)
+      listener->OnTimedOut(cb);
+
     return G_SOURCE_REMOVE;
   }
 
  private:
+  IEvent* listener_;
   amd_app_request_broker_result_cb cb_;
   void* user_data_;
   guint timer_ = 0;
 };
 
-class ClientChannel : public amd::ClientSocket {
+class ClientChannel : public amd::ClientSocket,
+                      public ResultCb::IEvent,
+                      public std::enable_shared_from_this<ClientChannel> {
  public:
-  ClientChannel(pid_t pid, uid_t uid)
+  ClientChannel(pid_t pid, uid_t uid, bool once = false)
       : ClientSocket(), pid_(pid), uid_(uid) {
     auto* channel = g_io_channel_unix_new(GetFd());
     if (channel == nullptr) {
@@ -125,6 +141,19 @@ class ClientChannel : public amd::ClientSocket {
     return uid_;
   }
 
+  bool IsOnce() const {
+    return once_;
+  }
+
+  std::shared_ptr<ClientChannel> GetSharedPtr() {
+    return shared_from_this();
+  }
+
+  void AddResultCb(unsigned int timeout, amd_app_request_broker_result_cb cb,
+      void* user_data) {
+    Push(std::make_shared<ResultCb>(this, timeout, cb, user_data));
+  }
+
   void Push(std::shared_ptr<ResultCb> result_cb) {
     queue_.push(std::move(result_cb));
   }
@@ -140,6 +169,11 @@ class ClientChannel : public amd::ClientSocket {
   }
 
  private:
+  void OnTimedOut(ResultCb* cb) override {
+    cb->Invoke(-ETIMEDOUT);
+    queue_.pop();
+  }
+
   static gboolean OnDataReceived(GIOChannel* channel, GIOCondition cond,
       gpointer user_data);
   static gboolean OnSocketDisconnected(GIOChannel* channel, GIOCondition cond,
@@ -148,6 +182,7 @@ class ClientChannel : public amd::ClientSocket {
  private:
   pid_t pid_;
   uid_t uid_;
+  bool once_;
   GIOChannel* channel_ = nullptr;
   guint source_ = 0;
   guint disconn_source_ = 0;
@@ -167,9 +202,10 @@ class AppRequestBroker {
     return inst;
   }
 
-  std::shared_ptr<ClientChannel> CreateClientChannel(pid_t pid, uid_t uid) {
+  std::shared_ptr<ClientChannel> CreateClientChannel(pid_t pid, uid_t uid,
+      bool once = false) {
     try {
-      auto client_channel = std::make_shared<ClientChannel>(pid, uid);
+      auto client_channel = std::make_shared<ClientChannel>(pid, uid, once);
       std::string endpoint = std::string(PATH_AUL_APPS) + "/" +
           std::to_string(uid) + "/" + std::to_string(pid) + "/.app-sock";
       client_channel->Connect(endpoint);
@@ -183,6 +219,15 @@ class AppRequestBroker {
     }
   }
 
+  void AddOneshotClientChannel(std::shared_ptr<ClientChannel> client_channel) {
+    oneshot_channels_.push_back(std::move(client_channel));
+  }
+
+  void RemoveOneshotClientChannel(
+      std::shared_ptr<ClientChannel> client_channel) {
+    oneshot_channels_.remove(client_channel);
+  }
+
   void AddClientChannel(pid_t pid,
       std::shared_ptr<ClientChannel> client_channel) {
     channels_[pid] = std::move(client_channel);
@@ -215,6 +260,7 @@ class AppRequestBroker {
   }
 
  private:
+  std::list<std::shared_ptr<ClientChannel>> oneshot_channels_;
   std::unordered_map<pid_t, std::shared_ptr<ClientChannel>> channels_;
   amd_app_request_broker_disconnected_event_cb disconn_cb_ = nullptr;
   void* disconn_data_ = nullptr;
@@ -234,6 +280,13 @@ gboolean ClientChannel::OnDataReceived(GIOChannel* channel, GIOCondition cond,
       result_cb->Invoke(res);
   }
 
+  if (handle->IsOnce()) {
+    handle->source_ = 0;
+    AppRequestBroker::GetInst().RemoveOneshotClientChannel(
+        handle->GetSharedPtr());
+    return G_SOURCE_REMOVE;
+  }
+
   if (res != 0) {
     _E("Error occurs. result(%d), pid(%d), fd(%d)",
         res, handle->GetPid(), handle->GetFd());
@@ -261,7 +314,13 @@ gboolean ClientChannel::OnSocketDisconnected(GIOChannel* channel,
   }
 
   handle->disconn_source_ = 0;
-  AppRequestBroker::GetInst().RemoveClientChannel(handle->GetPid());
+  if (handle->IsOnce()) {
+    AppRequestBroker::GetInst().RemoveOneshotClientChannel(
+        handle->GetSharedPtr());
+  } else {
+    AppRequestBroker::GetInst().RemoveClientChannel(handle->GetPid());
+  }
+
   return G_SOURCE_REMOVE;
 }
 
@@ -291,6 +350,11 @@ extern "C" EXPORT_API int amd_app_request_broker_send(
     if (!timeout_str.empty() && isdigit(timeout_str[0])) {
       timeout = static_cast<unsigned int>(std::stoi(timeout_str));
       _I("timeout: %ums", timeout);
+      channel = broker.CreateClientChannel(request->pid, request->uid, true);
+      if (channel == nullptr)
+        return -ECOMM;
+
+      broker.AddOneshotClientChannel(channel);
     }
 
     auto [raw, len] = b.ToRaw();
@@ -311,11 +375,15 @@ extern "C" EXPORT_API int amd_app_request_broker_send(
     }
 
     _E("Client channel will be removed. pid(%d)", channel->GetPid());
-    broker.RemoveClientChannel(channel->GetPid());
+    if (channel->IsOnce())
+      broker.RemoveOneshotClientChannel(channel);
+    else
+      broker.RemoveClientChannel(channel->GetPid());
+
     return ret;
   }
 
-  channel->Push(std::make_shared<ResultCb>(timeout, callback, user_data));
+  channel->AddResultCb(timeout, callback, user_data);
   return 0;
 }