From: Hwankyu Jhun <h.jhun@samsung.com>
Date: Wed, 31 Jul 2024 01:43:15 +0000 (+0900)
Subject: Modify ClientSocket implementation
X-Git-Tag: accepted/tizen/unified/20240801.150153~3
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=2571870cd1eec89f953c8b032a208dd58a8ff61e;p=platform%2Fcore%2Fappfw%2Frpc-port.git

Modify ClientSocket implementation

- Adds Send() and Receive() methods for sending the parcelable instance
- Uses recv() instead of read()
- Adds exception handlings about EAGAIN error
- Adds the retrying count for error handling

Change-Id: I41dd952d6fa90500a30f0193f0e8fe747675b462
Signed-off-by: Hwankyu Jhun <h.jhun@samsung.com>
---

diff --git a/src/rpc-port/client-socket-internal.cc b/src/rpc-port/client-socket-internal.cc
index 9a71b4e..c982e31 100644
--- a/src/rpc-port/client-socket-internal.cc
+++ b/src/rpc-port/client-socket-internal.cc
@@ -14,39 +14,54 @@
  * limitations under the License.
  */
 
+#include "rpc-port/client-socket-internal.hh"
+
 #include <errno.h>
 #include <fcntl.h>
+#include <glib-unix.h>
 #include <limits.h>
 #include <sys/socket.h>
 #include <sys/types.h>
 #include <sys/un.h>
 #include <unistd.h>
 
-#include "client-socket-internal.hh"
-#include "exception-internal.hh"
-#include "log-private.hh"
+#include "rpc-port/exception-internal.hh"
+#include "rpc-port/log-private.hh"
+
+namespace {
+
+const int kMaxRetryingCount = 10;
+
+class TemporaryBlockingMode {
+ public:
+  explicit TemporaryBlockingMode(int fd) : fd_(fd) {
+    g_unix_set_fd_nonblocking(fd_, false, nullptr);
+  }
+
+  ~TemporaryBlockingMode() { g_unix_set_fd_nonblocking(fd_, true, nullptr); }
+
+ private:
+  int fd_;
+};
+
+}  // namespace
 
 namespace rpc_port {
 namespace internal {
 
 ClientSocket::ClientSocket() {
   fd_ = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
-  if (fd_ < 0) {
-    // LCOV_EXCL_START
+  if (fd_ < 0) {  // LCOV_EXCL_START
     fd_ = -errno;
     _E("socket() is failed. errno(%d)", errno);
     THROW(fd_);
-    // LCOV_EXCL_STOP
-  }
+  }  // LCOV_EXCL_STOP
 }
 
-ClientSocket::ClientSocket(int fd) : fd_(fd) {
-  SetCloseOnExec();
-}
+ClientSocket::ClientSocket(int fd) : fd_(fd) { SetCloseOnExec(); }
 
 ClientSocket::~ClientSocket() {
-  if (!IsClosed())
-    Close();
+  if (!IsClosed()) Close();
 }
 
 void ClientSocket::SetCloseOnExec() {
@@ -72,31 +87,46 @@ int ClientSocket::Connect(const std::string& endpoint) {
       endpoint.c_str());
   struct sockaddr* sockaddr_ptr = reinterpret_cast<struct sockaddr*>(&sockaddr);
   socklen_t len = static_cast<socklen_t>(sizeof(sockaddr));
-  int ret = connect(GetFd(), sockaddr_ptr, len);
-  fcntl(GetFd(), F_SETFL, flag);
-  if (ret < 0) {
-    // LCOV_EXCL_START
+
+  int ret;
+  int retrying_count = 2;
+  do {
+    ret = connect(GetFd(), sockaddr_ptr, len);
+    if (ret == 0) break;
+
+    retrying_count--;
     ret = -errno;
+    usleep(100 * 1000);
+  } while (retrying_count > 0);
+
+  fcntl(GetFd(), F_SETFL, flag);
+  if (ret < 0) {  // LCOV_EXCL_START
     _E("connect() is failed. errno(%d)", errno);
     return ret;
-    // LCOV_EXCL_STOP
-  }
+  }  // LCOV_EXCL_STOP
 
   return 0;
 }
 
 int ClientSocket::Send(const void* buf, unsigned int size) {
+  int retrying_count = kMaxRetryingCount;
   const unsigned char* buffer = static_cast<const unsigned char*>(buf);
   size_t len = size;
   while (len) {
     ssize_t bytes = send(GetFd(), buffer, len, MSG_NOSIGNAL);
-    if (bytes < 0) {
-      // LCOV_EXCL_START
+    if (bytes < 0) {  // LCOV_EXCL_START
+      if (errno == EINTR || errno == EAGAIN) {
+        if (retrying_count > 0) {
+          retrying_count--;
+          usleep(10 * 1000);
+          continue;
+        }
+      }
+
       int ret = -errno;
       _E("send() is failed. fd(%d), errno(%d)", GetFd(), errno);
       return ret;
-      // LCOV_EXCL_STOP
-    }
+    }  // LCOV_EXCL_STOP
 
     len -= bytes;
     buffer += bytes;
@@ -106,23 +136,27 @@ int ClientSocket::Send(const void* buf, unsigned int size) {
 }
 
 int ClientSocket::Receive(void* buf, unsigned int size) {
+  int retyring_count = kMaxRetryingCount;
   unsigned char* buffer = static_cast<unsigned char*>(buf);
   size_t len = size;
   while (len) {
-    ssize_t bytes = read(GetFd(), buffer, len);
-    if (bytes == 0) {
-      _W("EOF. fd(%d)", GetFd());  // LCOV_EXCL_START
-      return -EIO;  // LCOV_EXCL_STOP
-    }
-
-    if (bytes < 0) {
-      if (errno == EINTR) {
-        usleep(100 * 1000);
-        continue;
+    ssize_t bytes = recv(GetFd(), buffer, len, 0);
+    if (bytes == 0) {  // LCOV_EXCL_START
+      _W("EOF. fd(%d)", GetFd());
+      return -EIO;
+    }  // LCOV_EXCL_STOP
+
+    if (bytes < 0) {  // LCOV_EXCL_START
+      if (errno == EINTR || errno == EAGAIN) {
+        if (retyring_count > 0) {
+          retyring_count--;
+          usleep(100 * 1000);
+          continue;
+        }
       }
 
-      return -errno;  // LCOV_EXCL_LINE
-    }
+      return -errno;
+    }  // LCOV_EXCL_STOP
 
     len -= bytes;
     buffer += bytes;
@@ -132,16 +166,15 @@ int ClientSocket::Receive(void* buf, unsigned int size) {
 }
 
 void ClientSocket::SetReceiveTimeout(int timeout) {
-  if (timeout == INT_MAX)
-    return;  // LCOV_EXCL_LINE
+  if (timeout == INT_MAX) return;  // LCOV_EXCL_LINE
 
   if (timeout == -1)
     timeout = 5000;
 
-  if (timeout < 0) {
-    _E("Invalid parameter");  // LCOV_EXCL_LINE
-    THROW(-EINVAL);  // LCOV_EXCL_LINE
-  }
+  if (timeout < 0) {  // LCOV_EXCL_START
+    _E("Invalid parameter");
+    THROW(-EINVAL);
+  }  // LCOV_EXCL_STOP
 
   struct timeval tv = {
     .tv_sec = static_cast<time_t>(timeout / 1000),
@@ -149,22 +182,16 @@ void ClientSocket::SetReceiveTimeout(int timeout) {
   };
   socklen_t len = static_cast<socklen_t>(sizeof(struct timeval));
   int ret = setsockopt(GetFd(), SOL_SOCKET, SO_RCVTIMEO, &tv, len);
-  if (ret < 0) {
-    // LCOV_EXCL_START
+  if (ret < 0) {  // LCOV_EXCL_START
     ret = -errno;
     _E("setsockopt() is failed. errno(%d)", errno);
     THROW(ret);
-    // LCOV_EXCL_STOP
-  }
+  }  // LCOV_EXCL_STOP
 }
 
-bool ClientSocket::IsClosed() {
-  return fd_ < 0 ? true : false;
-}
+bool ClientSocket::IsClosed() { return fd_ < 0; }
 
-int ClientSocket::GetFd() const {
-  return fd_;
-}
+int ClientSocket::GetFd() const { return fd_; }
 
 int ClientSocket::RemoveFd() {
   int fd = fd_;
@@ -172,9 +199,34 @@ int ClientSocket::RemoveFd() {
   return fd;
 }
 
-void ClientSocket::SetNonblock() {
-  int flag = fcntl(GetFd(), F_GETFL, 0);
-  fcntl(GetFd(), F_SETFL, flag | O_NONBLOCK);
+void ClientSocket::SetNonblock(bool nonblock) {
+  g_unix_set_fd_nonblocking(fd_, nonblock, nullptr);
+}
+
+int ClientSocket::Send(const tizen_base::Parcelable& parcelable) {
+  TemporaryBlockingMode mode(GetFd());
+  tizen_base::Parcel parcel;
+  parcel.WriteParcelable(parcelable);
+  size_t size = parcel.GetDataSize();
+  int ret = Send(reinterpret_cast<void*>(&size), sizeof(size));
+  if (ret != 0) return ret;
+
+  return Send(parcel.GetData(), size);
+}
+
+int ClientSocket::Receive(tizen_base::Parcelable* parcelable) {
+  TemporaryBlockingMode mode(GetFd());
+  size_t size = 0;
+  int ret = Receive(reinterpret_cast<void*>(&size), sizeof(size));
+  if (ret != 0) return ret;
+
+  std::vector<uint8_t> data(size);
+  ret = Receive(data.data(), size);
+  if (ret != 0) return ret;
+
+  tizen_base::Parcel parcel(data.data(), data.size(), true);
+  parcel.ReadParcelable(parcelable);
+  return ret;
 }
 
 }  // namespace internal
diff --git a/src/rpc-port/client-socket-internal.hh b/src/rpc-port/client-socket-internal.hh
index d033b2e..9548b43 100644
--- a/src/rpc-port/client-socket-internal.hh
+++ b/src/rpc-port/client-socket-internal.hh
@@ -19,6 +19,8 @@
 
 #include <string>
 
+#include <parcel.hh>
+
 namespace rpc_port {
 namespace internal {
 
@@ -36,9 +38,12 @@ class ClientSocket {
   bool IsClosed();
   int GetFd() const;
   int RemoveFd();
-  void SetNonblock();
+  void SetNonblock(bool nonblock = true);
   void SetCloseOnExec();
 
+  int Send(const tizen_base::Parcelable& parcelable);
+  int Receive(tizen_base::Parcelable* parcelable);
+
  private:
   int fd_;
 };
diff --git a/src/rpc-port/proxy-internal.cc b/src/rpc-port/proxy-internal.cc
index 4058aca..ce4dcc8 100644
--- a/src/rpc-port/proxy-internal.cc
+++ b/src/rpc-port/proxy-internal.cc
@@ -53,86 +53,6 @@ std::string GenInstance() {
   return std::string(uuid) + "@" + Aul::GetAppId(getpid());
 }
 
-int SendRequest(ClientSocket* client, const Request& request) {
-  tizen_base::Parcel parcel;
-  parcel.WriteParcelable(const_cast<Request&>(request));
-  size_t size = parcel.GetDataSize();
-  int ret = client->Send(reinterpret_cast<void*>(&size), sizeof(size));
-  if (ret != 0) {
-    // LCOV_EXCL_START
-    _E("Send() is failed. error(%d)", ret);
-    return -1;
-    // LCOV_EXCL_STOP
-  }
-
-  ret = client->Send(parcel.GetData(), size);
-  if (ret != 0) {
-    // LCOV_EXCL_START
-    _E("Send() is failed. error(%d)", ret);
-    return -1;
-    // LCOV_EXCL_STOP
-  }
-
-  return 0;
-}
-
-int ReceiveResponse(ClientSocket* client, Response** response) {
-  int flags = fcntl(client->GetFd(), F_GETFL, 0);
-  fcntl(client->GetFd(), F_SETFL, flags & ~O_NONBLOCK);
-
-  size_t size = 0;
-  int ret = client->Receive(reinterpret_cast<void*>(&size), sizeof(size));
-  if (ret != 0) {
-    // LCOV_EXCL_START
-    _E("Receive() is failed. error(%d)", ret);
-    fcntl(client->GetFd(), F_SETFL, flags);
-    return -1;
-    // LCOV_EXCL_STOP
-  }
-
-  uint8_t* buf = static_cast<uint8_t*>(malloc(size));
-  if (buf == nullptr) {
-    // LCOV_EXCL_START
-    _E("Out of memory");
-    fcntl(client->GetFd(), F_SETFL, flags);
-    return -1;
-    // LCOV_EXCL_STOP
-  }
-
-  ret = client->Receive(buf, size);
-  if (ret != 0) {
-    // LCOV_EXCL_START
-    _E("Receive() is failed. error(%d)", ret);
-    free(buf);
-    fcntl(client->GetFd(), F_SETFL, flags);
-    return -1;
-    // LCOV_EXCL_STOP
-  }
-
-  tizen_base::Parcel parcel(buf, size, false);
-  *response = new (std::nothrow) Response();
-  if (*response == nullptr) {
-    // LCOV_EXCL_START
-    _E("Out of memory");
-    fcntl(client->GetFd(), F_SETFL, flags);
-    return -1;
-    // LCOV_EXCL_STOP
-  }
-
-  parcel.ReadParcelable(*response);
-  fcntl(client->GetFd(), F_SETFL, flags);
-  return 0;
-}
-
-int ReceiveResult(ClientSocket* client) {
-  Response* response = nullptr;
-  int ret = ReceiveResponse(client, &response);
-  if (ret != 0) return ret;
-
-  std::unique_ptr<Response> response_auto(response);
-  return response->GetResult();
-}
-
 bool IsDaemon(const std::string& name) {
   if (name.compare(0, strlen(kDPrefix), kDPrefix) == 0) return true;
 
@@ -169,17 +89,16 @@ int Proxy::MainPortConnect(const std::string& instance, bool sync) {
     return RPC_PORT_ERROR_IO_ERROR;  // LCOV_EXCL_LINE
 
   Request request(instance.c_str(), kPortTypeMain);
-  int ret = SendRequest(main_client_.get(), request);
+  int ret = main_client_->Send(request);
   if (ret != 0) return RPC_PORT_ERROR_IO_ERROR;  // LCOV_EXCL_LINE
 
   main_client_->SetNonblock();
   if (sync) {
-    Response* response = nullptr;
-    ret = ReceiveResponse(main_client_.get(), &response);
+    Response response;
+    ret = main_client_->Receive(&response);
     if (ret != 0) return RPC_PORT_ERROR_IO_ERROR;  // LCOV_EXCL_LINE
 
-    std::unique_ptr<Response> response_auto(response);
-    if (response->GetResult() != 0) {
+    if (response.GetResult() != 0) {
       // LCOV_EXCL_START
       _E("Permission denied");
       return RPC_PORT_ERROR_PERMISSION_DENIED;
@@ -203,17 +122,16 @@ int Proxy::DelegatePortConnect(const std::string& instance, bool sync) {
     return RPC_PORT_ERROR_IO_ERROR;  // LCOV_EXCL_LINE
 
   Request request(instance.c_str(), kPortTypeDelegate);
-  int ret = SendRequest(delegate_client_.get(), request);
+  int ret = delegate_client_->Send(request);
   if (ret != 0) return RPC_PORT_ERROR_IO_ERROR;  // LCOV_EXCL_LINE
 
   delegate_client_->SetNonblock();
   if (sync) {
-    Response* response = nullptr;
-    ret = ReceiveResponse(delegate_client_.get(), &response);
+    Response response;
+    ret = delegate_client_->Receive(&response);
     if (ret != 0) return RPC_PORT_ERROR_IO_ERROR;  // LCOV_EXCL_LINE
 
-    std::unique_ptr<Response> response_auto(response);
-    if (response->GetResult() != 0) {
+    if (response.GetResult() != 0) {
       // LCOV_EXCL_START
       _E("Permission denied");
       return RPC_PORT_ERROR_PERMISSION_DENIED;
@@ -698,7 +616,9 @@ void Proxy::OnResponseReceived(int fd) {
     // LCOV_EXCL_STOP
   }
 
-  if (ReceiveResult(client.get()) != 0) {
+  Response response;
+  int ret = client->Receive(&response);
+  if (ret != 0 || response.GetResult() != 0) {
     _E("Permission denied");
     auto* listener = listener_;
     listener_ = nullptr;
diff --git a/src/rpc-port/request-internal.cc b/src/rpc-port/request-internal.cc
index c293c29..93bbcec 100644
--- a/src/rpc-port/request-internal.cc
+++ b/src/rpc-port/request-internal.cc
@@ -22,21 +22,15 @@ namespace rpc_port {
 namespace internal {
 
 Request::Request(std::string instance, std::string port_type)
-    : instance_(std::move(instance)),
-      port_type_(std::move(port_type)) {
-}
+    : instance_(std::move(instance)), port_type_(std::move(port_type)) {}
 
 void Request::SetPortType(std::string port_type) {
   port_type_ = std::move(port_type);
 }
 
-const std::string& Request::GetInstance() {
-  return instance_;
-}
+const std::string& Request::GetInstance() const { return instance_; }
 
-const std::string& Request::GetPortType() {
-  return port_type_;
-}
+const std::string& Request::GetPortType() const { return port_type_; }
 
 void Request::WriteToParcel(tizen_base::Parcel* parcel) const {
   parcel->WriteString(instance_);
diff --git a/src/rpc-port/request-internal.hh b/src/rpc-port/request-internal.hh
index 3aa9b05..8f40c16 100644
--- a/src/rpc-port/request-internal.hh
+++ b/src/rpc-port/request-internal.hh
@@ -32,8 +32,8 @@ class Request : public tizen_base::Parcelable {
   ~Request() = default;
 
   void SetPortType(std::string port_type);
-  const std::string& GetInstance();
-  const std::string& GetPortType();
+  const std::string& GetInstance() const;
+  const std::string& GetPortType() const;
 
   void WriteToParcel(tizen_base::Parcel* parcel) const override;
   void ReadFromParcel(tizen_base::Parcel* parcel) override;
diff --git a/src/rpc-port/response-internal.cc b/src/rpc-port/response-internal.cc
index 9d32604..1d3102f 100644
--- a/src/rpc-port/response-internal.cc
+++ b/src/rpc-port/response-internal.cc
@@ -19,15 +19,11 @@
 namespace rpc_port {
 namespace internal {
 
-Response::Response(int result) : result_(result) {
-}
+Response::Response(int result) : result_(result) {}
 
-Response::Response() : result_(0) {
-}
+Response::Response() : result_(0) {}
 
-int Response::GetResult() {
-  return result_;
-}
+int Response::GetResult() const { return result_; }
 
 void Response::WriteToParcel(tizen_base::Parcel* parcel) const {
   parcel->WriteInt32(result_);
diff --git a/src/rpc-port/response-internal.hh b/src/rpc-port/response-internal.hh
index 2c12ab2..ac80272 100644
--- a/src/rpc-port/response-internal.hh
+++ b/src/rpc-port/response-internal.hh
@@ -29,7 +29,7 @@ class Response : public tizen_base::Parcelable {
   Response();
   ~Response() = default;
 
-  int GetResult();
+  int GetResult() const;
 
   void WriteToParcel(tizen_base::Parcel* parcel) const override;
   void ReadFromParcel(tizen_base::Parcel* parcel) override;
diff --git a/src/rpc-port/stub-internal.cc b/src/rpc-port/stub-internal.cc
index aa2cecf..22f1060 100644
--- a/src/rpc-port/stub-internal.cc
+++ b/src/rpc-port/stub-internal.cc
@@ -44,51 +44,6 @@ constexpr const char kPortTypeMain[] = "main";
 constexpr const char kPortTypeDelegate[] = "delegate";
 constexpr uid_t kRegularUidMin = 5000;
 
-int ReceiveRequest(ClientSocket* client, Request** request) {
-  size_t size = 0;
-  int ret = client->Receive(reinterpret_cast<void*>(&size), sizeof(size));
-  if (ret != 0) {
-    _E("Receive() is failed. error(%d)", ret);  // LCOV_EXCL_LINE
-    return -1;  // LCOV_EXCL_LINE
-  }
-
-  std::vector<uint8_t> buf(size);
-  ret = client->Receive(buf.data(), size);
-  if (ret != 0) {
-    _E("Receive() is failed. error(%d)", ret);  // LCOV_EXCL_LINE
-    return -1;  // LCOV_EXCL_LINE
-  }
-
-  tizen_base::Parcel parcel(buf.data(), buf.size());
-  *request = new (std::nothrow) Request();
-  if (*request == nullptr) {
-    _E("Out of memory");  // LCOV_EXCL_LINE
-    return -1;  // LCOV_EXCL_LINE
-  }
-
-  parcel.ReadParcelable(*request);
-  return 0;
-}
-
-int SendResponse(ClientSocket* client, const Response& response) {
-  tizen_base::Parcel parcel;
-  parcel.WriteParcelable(const_cast<Response&>(response));
-  size_t size = parcel.GetDataSize();
-  int ret = client->Send(reinterpret_cast<void*>(&size), sizeof(size));
-  if (ret != 0) {
-    _E("Send() is failed. error(%d)", ret);  // LCOV_EXCL_LINE
-    return -1;  // LCOV_EXCL_LINE
-  }
-
-  ret = client->Send(parcel.GetData(), size);
-  if (ret != 0) {
-    _E("Send() is failed. error(%d)", ret);  // LCOV_EXCL_LINE
-    return -1;  // LCOV_EXCL_LINE
-  }
-
-  return 0;
-}
-
 }  // namespace
 
 std::unordered_set<Stub*> Stub::freed_stubs_;
@@ -343,7 +298,7 @@ void Stub::CheckPermission(const std::shared_ptr<Request>& request,
     if (pending_request_count_ != 0) pending_request_count_--;
 
     Response response(res);
-    int ret = SendResponse(client.get(), response);
+    int ret = client->Send(response);
     if (ret != 0) return;  // LCOV_EXCL_LINE
 
     if (res != 0) {
@@ -397,11 +352,10 @@ void Stub::OnRequestReceived(gint fd) {
     // LCOV_EXCL_STOP
   }
 
-  Request* request = nullptr;
-  int ret = ReceiveRequest(client.get(), &request);
+  auto request = std::make_shared<Request>();
+  int ret = client->Receive(request.get());
   if (ret != 0) return;  // LCOV_EXCL_LINE
 
-  std::shared_ptr<Request> request_auto(request);
   std::shared_ptr<PeerCred> cred(PeerCred::Get(client->GetFd()));
   if (!cred) {
     // LCOV_EXCL_START
@@ -410,7 +364,7 @@ void Stub::OnRequestReceived(gint fd) {
     // LCOV_EXCL_STOP
   }
 
-  CheckPermission(request_auto, client, cred);
+  CheckPermission(request, client, cred);
 }
 
 }  // namespace internal