Remove inefficient repeated event reception

If the socket connection is invalid, the event may continue to occur.
In this case, the event handler is removed to prevent unnecessary event reception.

Change-Id: I63851ac57b6f0820229ae4c0548af88988915061
Signed-off-by: Changgyu Choi <changyu.choi@samsung.com>

diff --git a/src/rpc-port/accepted-port-internal.cc b/src/rpc-port/accepted-port-internal.cc
index f8d2d16389429fd91a25a402cdc76df224405549..f018d164416e3130cd91698d1a0b670f77058bfc 100644 (file)
--- a/src/rpc-port/accepted-port-internal.cc
+++ b/src/rpc-port/accepted-port-internal.cc
@@ -78,7 +78,10 @@ gboolean AcceptedPort::UnixFdSourceFunc(gint fd, GIOCondition cond,
   if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL)) {
     listener->OnSocketDisconnected(fd);
   } else if (cond & G_IO_IN) {
-    listener->OnDataReceived(fd);
+    if (!listener->OnDataReceived(fd)) {
+      accepted_port->source_ = nullptr;
+      return G_SOURCE_REMOVE;
+    }
   }
 
   return G_SOURCE_CONTINUE;

diff --git a/src/rpc-port/accepted-port-internal.hh b/src/rpc-port/accepted-port-internal.hh
index cc77055727189bdfeeee5067e4b4baaf748dcc47..5f78bfe7e14f464b0eeb5ab811162ae9df35d52d 100644 (file)
--- a/src/rpc-port/accepted-port-internal.hh
+++ b/src/rpc-port/accepted-port-internal.hh
@@ -31,7 +31,7 @@ class AcceptedPort : public Port {
   class IEvent {
    public:
     virtual ~IEvent() = default;
-    virtual void OnDataReceived(int fd) = 0;
+    virtual bool OnDataReceived(int fd) = 0;
     virtual void OnSocketDisconnected(int fd) = 0;
   };
 

diff --git a/src/rpc-port/client-channel-internal.cc b/src/rpc-port/client-channel-internal.cc
index 345a7b01880009dcca4c6eb5f1c6b5f2c8af3114..e553bba32c532ebc15bdb442f6cde75f137828ac 100644 (file)
--- a/src/rpc-port/client-channel-internal.cc
+++ b/src/rpc-port/client-channel-internal.cc
@@ -91,10 +91,14 @@ gboolean ClientChannel::UnixFdSourceFunc(gint fd, GIOCondition cond,
   if (!channel->listener_) return G_SOURCE_REMOVE;
 
   auto* listener = channel->listener_;
-  if (cond & G_IO_IN)
-    listener->OnResponseReceived(fd);
-  else if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL))
+  if (cond & G_IO_IN) {
+    if (!listener->OnResponseReceived(fd)) {
+      channel->source_ = nullptr;
+      return G_SOURCE_REMOVE;
+    }
+  } else if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL)) {
     listener->OnChannelDisconnected(fd);
+  }
 
   return G_SOURCE_CONTINUE;
 }

diff --git a/src/rpc-port/client-channel-internal.hh b/src/rpc-port/client-channel-internal.hh
index 22f0b992ceea5a6cc2782e02beac9399d8cb27fd..c2006afdab511b2c822392e0f0c1dc9aa14327ca 100644 (file)
--- a/src/rpc-port/client-channel-internal.hh
+++ b/src/rpc-port/client-channel-internal.hh
@@ -32,7 +32,7 @@ class ClientChannel : public ClientSocket {
    public:
     virtual ~IEvent() = default;
     virtual void OnChannelDisconnected(int fd) = 0;
-    virtual void OnResponseReceived(int fd) = 0;
+    virtual bool OnResponseReceived(int fd) = 0;
   };
 
   explicit ClientChannel(IEvent* listener);

diff --git a/src/rpc-port/proxy-internal.cc b/src/rpc-port/proxy-internal.cc
index c36d785b405cfc02e313a15bbb12fa720237e9ea..a34ebfcdc096e66df4d8fb89a90133f884f3c13d 100644 (file)
--- a/src/rpc-port/proxy-internal.cc
+++ b/src/rpc-port/proxy-internal.cc
@@ -523,17 +523,17 @@ void Proxy::OnSocketDisconnected(int fd) {
   DebugPort::RemoveSession(fd);
 }
 
-void Proxy::OnDataReceived(int fd) {
+bool Proxy::OnDataReceived(int fd) {
   std::lock_guard<std::recursive_mutex> lock(GetMutex());
   auto* listener = listener_;
   if (listener == nullptr) {
     // LCOV_EXCL_START
     _E("Invalid context");
-    return;
+    return false;
     // LCOV_EXCL_STOP
   }
 
-  if (!delegate_port_ || delegate_port_->GetReadFd() != fd) return;
+  if (!delegate_port_ || delegate_port_->GetReadFd() != fd) return false;
 
   char buffer[4];
   if (recv(fd, buffer, sizeof(buffer), MSG_PEEK | MSG_DONTWAIT) == 0) {
@@ -545,10 +545,11 @@ void Proxy::OnDataReceived(int fd) {
       DebugPort::RemoveSession(fd);
       main_port_.reset();
     }
-    return;
+    return true;
   }
 
   listener->OnReceived(target_appid_);
+  return true;
 }
 
 // LCOV_EXCL_START
@@ -571,12 +572,12 @@ void Proxy::OnChannelDisconnected(int fd) {
 }
 // LCOV_EXCL_STOP
 
-void Proxy::OnResponseReceived(int fd) {
+bool Proxy::OnResponseReceived(int fd) {
   std::lock_guard<std::recursive_mutex> lock(GetMutex());
   UnsetConnTimer();
   if (listener_ == nullptr) {  // LCOV_EXCL_START
     _E("Invalid context");
-    return;
+    return false;
   }  // LCOV_EXCL_STOP
 
   bool is_read = false;
@@ -584,7 +585,7 @@ void Proxy::OnResponseReceived(int fd) {
   std::unique_ptr<ClientChannel> client = GetClient(fd, &is_read, &is_delegate);
   if (!client) {  // LCOV_EXCL_START
     _E("Unknown fd(%d)", fd);
-    return;
+    return false;
   }  // LCOV_EXCL_STOP
 
   Response response;
@@ -598,12 +599,13 @@ void Proxy::OnResponseReceived(int fd) {
     delegate_client_[0].reset();
     delegate_client_[1].reset();
     listener->OnRejected(target_appid_, RPC_PORT_ERROR_PERMISSION_DENIED);
-    return;
+    return true;
   }
 
   client->SetNonblock();
   int client_fd = client->RemoveFd();
   SetPort(client_fd, is_read, is_delegate);
+  return true;
 }
 
 std::shared_ptr<Proxy> Proxy::GetSharedPtr() { return shared_from_this(); }

diff --git a/src/rpc-port/proxy-internal.hh b/src/rpc-port/proxy-internal.hh
index 9ec8b578cc43c9a3243f075de7ba8a4e209b6a92..5e75188899ef1ebdb7470e41703c63098188b1f6 100644 (file)
--- a/src/rpc-port/proxy-internal.hh
+++ b/src/rpc-port/proxy-internal.hh
@@ -63,9 +63,9 @@ class Proxy : public std::enable_shared_from_this<Proxy>,
   void OnFileCreated(const std::string& path) override;
   void OnFileDeleted(const std::string& path) override;
   void OnChannelDisconnected(int fd) override;
-  void OnResponseReceived(int fd) override;
+  bool OnResponseReceived(int fd) override;
   void OnSocketDisconnected(int fd) override;
-  void OnDataReceived(int fd) override;
+  bool OnDataReceived(int fd) override;
 
   static gboolean OnTimedOut(gpointer user_data);
   static gboolean OnIdle(gpointer user_data);

diff --git a/src/rpc-port/proxy-port-internal.cc b/src/rpc-port/proxy-port-internal.cc
index 1bfd5895dda744268f2880ffc9975d3eb6a3113e..2063780132b67fb7dcd635a95c2e737e72d47206 100644 (file)
--- a/src/rpc-port/proxy-port-internal.cc
+++ b/src/rpc-port/proxy-port-internal.cc
@@ -70,10 +70,14 @@ gboolean ProxyPort::UnixFdSourceFunc(gint fd, GIOCondition cond,
   if (!proxy_port->listener_) return G_SOURCE_REMOVE;
 
   auto* listener = proxy_port->listener_;
-  if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP))
+  if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
     listener->OnSocketDisconnected(fd);
-  else if (cond & G_IO_IN)
-    listener->OnDataReceived(fd);
+  } else if (cond & G_IO_IN) {
+    if (!listener->OnDataReceived(fd)) {
+      proxy_port->source_ = nullptr;
+      return G_SOURCE_REMOVE;
+    }
+  }
 
   return G_SOURCE_CONTINUE;
 }

diff --git a/src/rpc-port/proxy-port-internal.hh b/src/rpc-port/proxy-port-internal.hh
index e78bbba401b9798ee1acf9bb58f07177be0538ee..cbbc2d0dc798b13be93c126435591aa8fd5eebb6 100644 (file)
--- a/src/rpc-port/proxy-port-internal.hh
+++ b/src/rpc-port/proxy-port-internal.hh
@@ -33,7 +33,7 @@ class ProxyPort : public Port {
   class IEvent {
    public:
     virtual ~IEvent() = default;
-    virtual void OnDataReceived(int fd) = 0;
+    virtual bool OnDataReceived(int fd) = 0;
     virtual void OnSocketDisconnected(int fd) = 0;
   };
 

diff --git a/src/rpc-port/rpc-port.cc b/src/rpc-port/rpc-port.cc
index e80ed30ade4e994e3b609c01ec3394d93a4f6640..77b905092da34bbb42e4846051fc77eb30ee800d 100644 (file)
--- a/src/rpc-port/rpc-port.cc
+++ b/src/rpc-port/rpc-port.cc
@@ -638,4 +638,4 @@ RPC_API int rpc_port_stub_has_pending_request(rpc_port_stub_h h,
   auto* stub = static_cast<::StubExt*>(h);
   *has_request = stub->HasPendingRequest();
   return RPC_PORT_ERROR_NONE;
-}
\ No newline at end of file
+}

diff --git a/src/rpc-port/server-internal.cc b/src/rpc-port/server-internal.cc
index 957d1de0bcb0b225c93efde3b552948c62db85cd..9372aa3a88363406f97efe8fa9bf4f759fda58d5 100644 (file)
--- a/src/rpc-port/server-internal.cc
+++ b/src/rpc-port/server-internal.cc
@@ -44,7 +44,13 @@ gboolean Server::OnRequestReceived(gint fd, GIOCondition cond,
                                    gpointer user_data) {
   auto* server = static_cast<Server*>(user_data);
   auto* listener = server->listener_;
-  if (listener) listener->OnRequestReceived(fd);
+  if (listener) {
+    bool ret = listener->OnRequestReceived(fd);
+    if (!ret) {
+      GLib::SourceDestroy(server->source_);
+      server->source_ = nullptr;
+    }
+  }
 
   return G_SOURCE_CONTINUE;
 }

diff --git a/src/rpc-port/server-internal.hh b/src/rpc-port/server-internal.hh
index 48a8802012b4f2f81df2e1548f62b74da08ed190..3983d7731e49958c0ec7824fd17b1ac4f912921d 100644 (file)
--- a/src/rpc-port/server-internal.hh
+++ b/src/rpc-port/server-internal.hh
@@ -29,7 +29,7 @@ class Server : public ServerSocket {
   class IEvent {
    public:
     virtual ~IEvent() = default;
-    virtual void OnRequestReceived(int fd) = 0;
+    virtual bool OnRequestReceived(int fd) = 0;
   };
 
   Server(int fd, IEvent* listener);

diff --git a/src/rpc-port/stub-internal.cc b/src/rpc-port/stub-internal.cc
index 879638ea67cf26d8c56a0c09d191f2ce38333440..b6314305697ade451b6862d99f4f1c0e55f453c6 100644 (file)
--- a/src/rpc-port/stub-internal.cc
+++ b/src/rpc-port/stub-internal.cc
@@ -205,12 +205,12 @@ bool Stub::HasPendingRequest() const {
 
 GMainContext* Stub::GetMainContext() const { return context_; }
 
-void Stub::OnDataReceived(int fd) {
+bool Stub::OnDataReceived(int fd) {
   std::lock_guard<std::recursive_mutex> lock(GetMutex());
   if (listener_ == nullptr) {
     // LCOV_EXCL_START
     _E("Invalid context");
-    return;
+    return false;
     // LCOV_EXCL_STOP
   }
 
@@ -222,7 +222,7 @@ void Stub::OnDataReceived(int fd) {
         listener_->OnDisconnected(p->GetId(), p->GetInstance());
         RemoveAcceptedPorts(p->GetInstance());
         Aul::NotifyRpcFinished();
-        return;
+        return true;
       }
 
       int ret = listener_->OnReceived(p->GetId(), p->GetInstance(), p.get());
@@ -231,12 +231,14 @@ void Stub::OnDataReceived(int fd) {
         listener_->OnDisconnected(p->GetId(), p->GetInstance());
         RemoveAcceptedPorts(p->GetInstance());
         Aul::NotifyRpcFinished();
-        return;
+        return true;
       }
 
       break;
     }
   }
+
+  return true;
 }
 
 void Stub::OnSocketDisconnected(gint fd) {
@@ -351,12 +353,12 @@ void Stub::CheckPermission(const std::shared_ptr<Request>& request,
   response_func(res);
 }
 
-void Stub::OnRequestReceived(gint fd) {
+bool Stub::OnRequestReceived(gint fd) {
   std::lock_guard<std::recursive_mutex> lock(GetMutex());
   if (listener_ == nullptr) {
     // LCOV_EXCL_START
     _E("Invalid context");
-    return;
+    return false;
     // LCOV_EXCL_STOP
   }
 
@@ -364,23 +366,24 @@ void Stub::OnRequestReceived(gint fd) {
   if (!client) {
     // LCOV_EXCL_START
     _E("Out of memory");
-    return;
+    return false;
     // LCOV_EXCL_STOP
   }
 
   auto request = std::make_shared<Request>();
   int ret = client->Receive(request.get());
-  if (ret != 0) return;  // LCOV_EXCL_LINE
+  if (ret != 0) return false;  // LCOV_EXCL_LINE
 
   std::shared_ptr<PeerCred> cred(PeerCred::Get(client->GetFd()));
   if (!cred) {
     // LCOV_EXCL_START
     _E("Failed to create peer credentials");
-    return;
+    return false;
     // LCOV_EXCL_STOP
   }
 
   CheckPermission(request, client, cred);
+  return true;
 }
 
 }  // namespace internal

diff --git a/src/rpc-port/stub-internal.hh b/src/rpc-port/stub-internal.hh
index e6100d1f51cc4412e516e1153a46e6607945a6b8..e7b292e14196ef1f1a6351462d8a51881393f598 100644 (file)
--- a/src/rpc-port/stub-internal.hh
+++ b/src/rpc-port/stub-internal.hh
@@ -75,9 +75,9 @@ class Stub : public Server::IEvent, public AcceptedPort::IEvent {
   void CheckPermission(const std::shared_ptr<Request>& request,
                        const std::shared_ptr<ClientSocket>& client,
                        const std::shared_ptr<PeerCred>& cred);
-  void OnRequestReceived(int fd) override;
+  bool OnRequestReceived(int fd) override;
   void OnSocketDisconnected(int fd) override;
-  void OnDataReceived(int fd) override;
+  bool OnDataReceived(int fd) override;
 
  private:
   std::shared_ptr<AccessController> access_controller_ =