From 46fa11af8a408ea2f0dd531f716d40f3abb0dbc9 Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Mon, 13 Sep 2021 16:14:05 +0900 Subject: [PATCH 01/16] Fix Disconnect() method of Port Before closing the file descriptor, the method calls IgnoreIOEvent() to release the GSource. Change-Id: I8e7c91de98e1db838def468564ca98117d1985eb Signed-off-by: Hwankyu Jhun --- src/port-internal.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/port-internal.cc b/src/port-internal.cc index 4cceada..582e6a5 100644 --- a/src/port-internal.cc +++ b/src/port-internal.cc @@ -80,6 +80,8 @@ Port::~Port() { } void Port::Disconnect() { + IgnoreIOEvent(); + if (fd_ > 0) { _W("Close fd(%d)", fd_); close(fd_); -- 2.7.4 From 470a010b53731002d86c4ee014e985ec820e1cca Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Mon, 13 Sep 2021 16:51:35 +0900 Subject: [PATCH 02/16] Fix rpc_port_read() function Currently, calling Read() is failed, the port will be disconnected. The header of the parcel is added to check the procotol validation. This patch removes to call Disconnect() method when the Read() is failed. Change-Id: I070d9e361917a0bee6a0cd2f673f7157306af8cb Signed-off-by: Hwankyu Jhun --- src/rpc-port.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/rpc-port.cc b/src/rpc-port.cc index f4c5a35..2ed8c89 100644 --- a/src/rpc-port.cc +++ b/src/rpc-port.cc @@ -232,14 +232,12 @@ RPC_API int rpc_port_read(rpc_port_h h, void* buf, unsigned int size) { int ret = port->Read(reinterpret_cast(&seq), sizeof(seq)); if (ret < 0) { _E("IO Error"); - port->Disconnect(); return ret; } ret = port->Read(buf, size); if (ret < 0) { _E("IO Error"); - port->Disconnect(); return ret; } -- 2.7.4 From ab6ead108a65959ba85294714e6d13a6c77940e0 Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Mon, 13 Sep 2021 17:46:30 +0900 Subject: [PATCH 03/16] Release version 1.12.1 Changes: - Fix Disconnect() method of Port - Fix rpc_port_read() function Change-Id: I6132825793cf726538d92bb4cef22fadba0081db Signed-off-by: Hwankyu Jhun --- packaging/rpc-port.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packaging/rpc-port.spec b/packaging/rpc-port.spec index 0eaea19..b824f87 100644 --- a/packaging/rpc-port.spec +++ b/packaging/rpc-port.spec @@ -1,6 +1,6 @@ Name: rpc-port Summary: RPC Port library -Version: 1.12.0 +Version: 1.12.1 Release: 0 Group: Application Framework/Libraries License: Apache-2.0 -- 2.7.4 From c72a00774425d688a04cd8d0d43b0c693f1426b2 Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Tue, 14 Sep 2021 19:14:36 +0900 Subject: [PATCH 04/16] Remove GDBus dependency from rpc-port library The rpc-port was used the GDBusConnection to notify the presence of debug port. After this patch is applied, the rpc-port library doesn't use the GDBusConnection. The thread of the debug-port tries to connect to the debug port of the tool automatically if it's not connected. Change-Id: Ife29d7960360e9da21819b16993d406750a4d990 Signed-off-by: Hwankyu Jhun --- src/debug-port-internal.cc | 156 +++++++-------------------------------------- src/debug-port-internal.hh | 13 ---- utils/debug-port.cc | 107 ++----------------------------- utils/debug-port.hh | 14 ---- utils/main.cc | 3 - 5 files changed, 26 insertions(+), 267 deletions(-) diff --git a/src/debug-port-internal.cc b/src/debug-port-internal.cc index ab660a3..8e182fe 100644 --- a/src/debug-port-internal.cc +++ b/src/debug-port-internal.cc @@ -30,10 +30,6 @@ namespace internal { namespace { const char PATH_RPC_PORT_UTIL_SOCK[] = "/run/aul/daemons/.rpc-port-util-sock"; -const char RPC_PORT_SIGNAL_PATH[] = "/Org/Tizen/RPC/Port/Signal"; -const char RPC_PORT_SIGNAL_INTERFACE[] = "org.tizen.rpc.port.signal"; -const char RPC_PORT_SIGNAL_DEBUG[] = "Debug"; -const char RPC_PORT_SIGNAL_NEW[] = "New"; } // namespace std::atomic DebugPort::inst_; @@ -64,7 +60,6 @@ DebugPort* DebugPort::GetInst() { void DebugPort::Dispose() { Unwatch(); - Unsubscribe(); JoinThread(); disposed_ = true; } @@ -108,9 +103,6 @@ std::shared_ptr DebugPort::FindSession(int port) { int DebugPort::Send(int port, bool is_read, uint32_t seq, const void* buf, unsigned int size) { std::lock_guard lock(GetMutex()); - if (!IsConnected()) - return -1; - auto session = FindSession(port); if (session.get() == nullptr) { _E("Failed to find session. port(%d)", port); @@ -134,9 +126,7 @@ int DebugPort::Send(int port, bool is_read, uint32_t seq, } void DebugPort::Init() { - Subscribe(); - EmitSignal(RPC_PORT_SIGNAL_NEW); - is_running_ = false; + CreateThread(); disposed_ = false; } @@ -184,15 +174,15 @@ int DebugPort::Watch(int fd) { } void DebugPort::Unwatch() { - if (watch_tag_) { - g_source_remove(watch_tag_); - watch_tag_ = 0; - } - if (io_) { g_io_channel_unref(io_); io_ = nullptr; } + + if (watch_tag_) { + g_source_remove(watch_tag_); + watch_tag_ = 0; + } } gboolean DebugPort::OnDebugPortDisconnectedCb(GIOChannel* io, @@ -208,122 +198,6 @@ gboolean DebugPort::OnDebugPortDisconnectedCb(GIOChannel* io, return G_SOURCE_REMOVE; } -GDBusConnection* DebugPort::GetConnection() { - if (conn_) - return conn_; - - GError* error = nullptr; - conn_ = g_bus_get_sync(G_BUS_TYPE_SYSTEM, nullptr, &error); - if (conn_ == nullptr) { - _E("g_bus_get_sync() is failed. error(%s)", - error ? error->message : "unknown"); - g_clear_error(&error); - return nullptr; - } - - return conn_; -} - -void DebugPort::Subscribe() { - GDBusConnection* conn = GetConnection(); - if (conn == nullptr) - return; - - subs_tag_ = g_dbus_connection_signal_subscribe(conn, - nullptr, - RPC_PORT_SIGNAL_INTERFACE, - RPC_PORT_SIGNAL_DEBUG, - RPC_PORT_SIGNAL_PATH, - nullptr, - G_DBUS_SIGNAL_FLAGS_NONE, - OnGDBusSignalCb, - this, - nullptr); - if (subs_tag_ == 0) { - _E("g_dbus_connection_signal_subscribe() is failed"); - return; - } - - _D("tag(%u)", subs_tag_); -} - -void DebugPort::Unsubscribe() { - if (subs_tag_) - g_dbus_connection_signal_unsubscribe(conn_, subs_tag_); - - if (conn_) - g_object_unref(conn_); -} - -void DebugPort::EmitSignal(std::string signal) { - GDBusConnection* conn = GetConnection(); - if (conn == nullptr) - return; - - GError* error = nullptr; - gboolean ret = g_dbus_connection_emit_signal(conn, - nullptr, - RPC_PORT_SIGNAL_PATH, - RPC_PORT_SIGNAL_INTERFACE, - signal.c_str(), - nullptr, - &error); - if (ret != TRUE) { - _E("g_dbus_connection_emit_signal() is failed. error(%s)", - error ? error->message : "unknown"); - g_clear_error(&error); - return; - } - - ret = g_dbus_connection_flush_sync(conn, nullptr, &error); - if (ret != TRUE) { - _E("g_dbus_connection_flush_sync() is failed. error(%s)", - error ? error->message : "unknown"); - g_clear_error(&error); - return; - } - - _W("EmitSignal(%s)", signal.c_str()); -} - -void DebugPort::OnGDBusSignalCb(GDBusConnection *connection, - const gchar* sender_name, - const gchar* object_path, - const gchar* interface_name, - const gchar* signal_name, - GVariant* parameters, - gpointer user_data) { - _W("signal_name(%s)", signal_name); - std::string signal(signal_name); - if (signal != RPC_PORT_SIGNAL_DEBUG) - return; - - gchar* port_name = nullptr; - gint pid = -1; - g_variant_get(parameters, "(&si)", &port_name, &pid); - _W("port_name(%s), pid(%d)", port_name, pid); - - if (pid != 0 && pid != getpid()) { - _W("Invalid pid(%d)", pid); - return; - } - - auto* debug_port = static_cast(user_data); - int fd = debug_port->Connect(); - if (fd < 0) - return; - - std::lock_guard lock(debug_port->GetMutex()); - debug_port->port_.reset(new Port(fd, signal)); - int ret = debug_port->Watch(fd); - if (ret < 0) - return; - - debug_port->CreateThread(); - debug_port->SetConnectionStatus(true); - _W("Connected"); -} - void DebugPort::SetConnectionStatus(bool status) { std::lock_guard lock(GetMutex()); connected_ = status; @@ -344,8 +218,22 @@ void DebugPort::CreateThread() { break; } - if (!IsConnected()) - continue; + if (!IsConnected()) { + int fd = Connect(); + if (fd < 0) + continue; + + { + std::lock_guard lock(GetMutex()); + port_.reset(new Port(fd, "Debug")); + int ret = Watch(fd); + if (ret < 0) + continue; + + SetConnectionStatus(true); + _W("Connected"); + } + } int ret = port_->Write(reinterpret_cast(&len), sizeof(len)); if (ret < 0) { diff --git a/src/debug-port-internal.hh b/src/debug-port-internal.hh index ac63f41..06ccdd4 100644 --- a/src/debug-port-internal.hh +++ b/src/debug-port-internal.hh @@ -99,10 +99,6 @@ class DebugPort { int Connect(); int Watch(int fd); void Unwatch(); - GDBusConnection* GetConnection(); - void Subscribe(); - void Unsubscribe(); - void EmitSignal(std::string signal); void SetConnectionStatus(bool status); void CreateThread(); void JoinThread(); @@ -111,19 +107,10 @@ class DebugPort { static gboolean OnDebugPortDisconnectedCb(GIOChannel* io, GIOCondition cond, gpointer data); - static void OnGDBusSignalCb(GDBusConnection *connection, - const gchar* sender_name, - const gchar* object_path, - const gchar* interface_name, - const gchar* signal_name, - GVariant* parameters, - gpointer user_data); private: bool disposed_ = true; bool connected_ = false; - GDBusConnection* conn_ = nullptr; - guint subs_tag_ = 0; std::unique_ptr port_; GIOChannel* io_ = nullptr; guint watch_tag_ = 0; diff --git a/utils/debug-port.cc b/utils/debug-port.cc index 202d9da..9e8388a 100644 --- a/utils/debug-port.cc +++ b/utils/debug-port.cc @@ -31,13 +31,12 @@ namespace rpc_port { namespace util { +namespace { -static const char PATH_RPC_PORT_UTIL_SOCK[] = +constexpr const char PATH_RPC_PORT_UTIL_SOCK[] = "/run/aul/daemons/.rpc-port-util-sock"; -static const char RPC_PORT_SIGNAL_PATH[] = "/Org/Tizen/RPC/Port/Signal"; -static const char RPC_PORT_SIGNAL_INTERFACE[] = "org.tizen.rpc.port.signal"; -static const char RPC_PORT_SIGNAL_DEBUG[] = "Debug"; -static const char RPC_PORT_SIGNAL_NEW[] = "New"; + +} // namespace class DisconnectedEvent { public: @@ -66,8 +65,6 @@ class DisconnectedEvent { DebugPort::DebugPort(std::string port_name, int pid) : port_name_(std::move(port_name)), pid_(pid) { - Subscribe(); - int fd = CreateSocket(); if (fd < 0) { _E("Failed to create socket"); @@ -83,86 +80,6 @@ DebugPort::~DebugPort() { if (fd_ > 0) close(fd_); - - Unsubscribe(); -} - -void DebugPort::EmitSignal() { - GDBusConnection* conn = GetConnection(); - if (conn == nullptr) - return; - - GError* error = nullptr; - gboolean ret = g_dbus_connection_emit_signal(conn, - nullptr, - RPC_PORT_SIGNAL_PATH, - RPC_PORT_SIGNAL_INTERFACE, - RPC_PORT_SIGNAL_DEBUG, - g_variant_new("(si)", port_name_.c_str(), pid_), - &error); - if (ret != TRUE) { - _E("g_dbus_connection_emit_signal() is failed. error(%s)", - error ? error->message : "Unknown"); - g_clear_error(&error); - return; - } - - ret = g_dbus_connection_flush_sync(conn, nullptr, &error); - if (ret != TRUE) { - _E("g_dbus_connection_flush_sync() is failed. error(%s)", - error ? error->message : "Unknown"); - g_clear_error(&error); - return; - } - - _W("EmitSignal"); -} - -GDBusConnection* DebugPort::GetConnection() { - if (conn_) - return conn_; - - GError* error = nullptr; - conn_ = g_bus_get_sync(G_BUS_TYPE_SYSTEM, nullptr, &error); - if (conn_ == nullptr) { - _E("g_bus_get_sync() is failed. error(%s)", - error ? error->message : "Unknown"); - g_clear_error(&error); - return nullptr; - } - - return conn_; -} - -void DebugPort::Subscribe() { - GDBusConnection* conn = GetConnection(); - if (conn == nullptr) - return; - - subs_tag_ = g_dbus_connection_signal_subscribe(conn, - nullptr, - RPC_PORT_SIGNAL_INTERFACE, - RPC_PORT_SIGNAL_NEW, - RPC_PORT_SIGNAL_PATH, - nullptr, - G_DBUS_SIGNAL_FLAGS_NONE, - OnGDBusSignalCb, - this, - nullptr); - if (subs_tag_ == 0) { - _E("g_dbus_connection_signal_subscribe() is failed"); - return; - } - - _W("tag(%u)", subs_tag_); -} - -void DebugPort::Unsubscribe() { - if (subs_tag_) - g_dbus_connection_signal_unsubscribe(conn_, subs_tag_); - - if (conn_) - g_object_unref(conn_); } int DebugPort::CreateSocket() { @@ -267,22 +184,6 @@ void DebugPort::OnDisconnected(int pid, int fd) { }, event); } -void DebugPort::OnGDBusSignalCb(GDBusConnection *connection, - const gchar* sender_name, - const gchar* object_path, - const gchar* interface_name, - const gchar* signal_name, - GVariant* parameters, - gpointer user_data) { - _E("signal_name(%s)", signal_name); - std::string signal(signal_name); - if (signal != RPC_PORT_SIGNAL_NEW) - return; - - auto* debug_port = static_cast(user_data); - debug_port->EmitSignal(); -} - int DebugPort::Watch(int fd) { GIOChannel* io = g_io_channel_unix_new(fd); if (io == nullptr) { diff --git a/utils/debug-port.hh b/utils/debug-port.hh index 5c85bb4..836f6bc 100644 --- a/utils/debug-port.hh +++ b/utils/debug-port.hh @@ -38,12 +38,7 @@ class DebugPort : public Logger::IEvent { DebugPort(std::string port_name, int pid); virtual ~DebugPort(); - void EmitSignal(); - private: - GDBusConnection* GetConnection(); - void Subscribe(); - void Unsubscribe(); int CreateSocket(); int Watch(int fd); void Unwatch(); @@ -54,13 +49,6 @@ class DebugPort : public Logger::IEvent { void OnDataReceived(int pid, rpc_port_parcel_h parcel) override; void OnDisconnected(int pid, int fd) override; - static void OnGDBusSignalCb(GDBusConnection *connection, - const gchar* sender_name, - const gchar* object_path, - const gchar* interface_name, - const gchar* signal_name, - GVariant* parameters, - gpointer user_data); static gboolean OnDebugPortConnectedCb(GIOChannel* io, GIOCondition cond, gpointer data); @@ -72,8 +60,6 @@ class DebugPort : public Logger::IEvent { private: std::string port_name_; int pid_; - GDBusConnection* conn_ = nullptr; - guint subs_tag_ = 0; int fd_ = -1; GIOChannel* io_ = nullptr; guint watch_tag_ = 0; diff --git a/utils/main.cc b/utils/main.cc index d038565..68559fe 100644 --- a/utils/main.cc +++ b/utils/main.cc @@ -57,10 +57,7 @@ int main(int argc, char** argv) { } DebugPort debug_port(options->GetPortName(), options->GetPid()); - debug_port.EmitSignal(); - MainLoop loop; loop.Run(); - return 0; } -- 2.7.4 From 5015bfe93ba0ec8362363db09068644f248be55d Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Thu, 16 Sep 2021 09:19:52 +0900 Subject: [PATCH 05/16] Fix locking the mutex of the Port class To reduce the interval of locking the mutex, the mutex lock in the Read() method ie separated using braces. And, the mutex lock is added to the Disconnect() method. Change-Id: I60657385bc73cb988de0f53433bb1d22a21dad74 Signed-off-by: Hwankyu Jhun --- src/port-internal.cc | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/port-internal.cc b/src/port-internal.cc index 582e6a5..88d7722 100644 --- a/src/port-internal.cc +++ b/src/port-internal.cc @@ -82,6 +82,7 @@ Port::~Port() { void Port::Disconnect() { IgnoreIOEvent(); + std::lock_guard lock(mutex_); if (fd_ > 0) { _W("Close fd(%d)", fd_); close(fd_); @@ -118,17 +119,25 @@ int Port::Read(void* buf, unsigned int size) { int bytes_read = 0; char* buffer = static_cast(buf); int max_timeout = MAX_CNT * MAX_SLEEP; /* 10 sec */ - std::lock_guard lock(mutex_); + int fd; - if (fd_ < 0 || fd_ >= sysconf(_SC_OPEN_MAX)) { - _E("Invalid fd(%d)", fd_); - return RPC_PORT_ERROR_IO_ERROR; + { + std::lock_guard lock(mutex_); + fd = fd_; + if (fd < 0 || fd >= sysconf(_SC_OPEN_MAX)) { + _E("Invalid fd(%d)", fd); + return RPC_PORT_ERROR_IO_ERROR; + } } while (left) { - nb = read(fd_, buffer, left); + { + std::lock_guard lock(mutex_); + nb = read(fd, buffer, left); + } + if (nb == 0) { - _E("read_socket: ...read EOF, socket closed %d: nb %zd\n", fd_, nb); + _E("read_socket: ...read EOF, socket closed %d: nb %zd\n", fd, nb); return RPC_PORT_ERROR_IO_ERROR; } else if (nb == -1) { if (errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK) { @@ -136,7 +145,7 @@ int Port::Read(void* buf, unsigned int size) { nanosleep(&TRY_SLEEP_TIME, 0); max_timeout -= (TRY_SLEEP_TIME.tv_nsec / (BASE_SLEEP)); if (max_timeout <= 0) { - _E("read_socket: ...timed out fd %d: errno %d", fd_, errno); + _E("read_socket: ...timed out fd %d: errno %d", fd, errno); return RPC_PORT_ERROR_IO_ERROR; } TRY_SLEEP_TIME.tv_nsec *= 2; @@ -145,7 +154,7 @@ int Port::Read(void* buf, unsigned int size) { continue; } - _E("read_socket: ...error fd %d: errno %d\n", fd_, errno); + _E("read_socket: ...error fd %d: errno %d\n", fd, errno); return RPC_PORT_ERROR_IO_ERROR; } -- 2.7.4 From 871e71b25250e761430adf2d871e5a37a726cab0 Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Thu, 16 Sep 2021 10:32:51 +0900 Subject: [PATCH 06/16] Release version 1.12.2 Changes: - Fix locking the mutex of the Port class - Remove GDBus dependency from rpc-port library Change-Id: I87b75981d8ee402207189bf28ebeab4cb77ecf8d Signed-off-by: Hwankyu Jhun --- packaging/rpc-port.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packaging/rpc-port.spec b/packaging/rpc-port.spec index b824f87..5cb0832 100644 --- a/packaging/rpc-port.spec +++ b/packaging/rpc-port.spec @@ -1,6 +1,6 @@ Name: rpc-port Summary: RPC Port library -Version: 1.12.1 +Version: 1.12.2 Release: 0 Group: Application Framework/Libraries License: Apache-2.0 -- 2.7.4 From d7c8856363e2431b03515957fa0154ec52324107 Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Thu, 16 Sep 2021 11:36:50 +0900 Subject: [PATCH 07/16] Change file name 'message_sending_thread' is changed to 'message-sending-thread-internal'. Change-Id: Ie899d980c17283ccc48967b17566f3906ec3067f Signed-off-by: Hwankyu Jhun --- src/{message_sending_thread.cc => message-sending-thread-internal.cc} | 2 +- src/{message_sending_thread.hh => message-sending-thread-internal.hh} | 0 src/port-internal.cc | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename src/{message_sending_thread.cc => message-sending-thread-internal.cc} (98%) rename src/{message_sending_thread.hh => message-sending-thread-internal.hh} (100%) diff --git a/src/message_sending_thread.cc b/src/message-sending-thread-internal.cc similarity index 98% rename from src/message_sending_thread.cc rename to src/message-sending-thread-internal.cc index ef80356..470c1f5 100644 --- a/src/message_sending_thread.cc +++ b/src/message-sending-thread-internal.cc @@ -20,7 +20,7 @@ #include #include "log-private.hh" -#include "message_sending_thread.hh" +#include "message-sending-thread-internal.hh" namespace rpc_port { namespace internal { diff --git a/src/message_sending_thread.hh b/src/message-sending-thread-internal.hh similarity index 100% rename from src/message_sending_thread.hh rename to src/message-sending-thread-internal.hh diff --git a/src/port-internal.cc b/src/port-internal.cc index 88d7722..77120f5 100644 --- a/src/port-internal.cc +++ b/src/port-internal.cc @@ -27,7 +27,7 @@ #include "include/rpc-port.h" #include "log-private.hh" -#include "message_sending_thread.hh" +#include "message-sending-thread-internal.hh" #include "port-internal.hh" #define MAX_CNT 100 -- 2.7.4 From 25e42b0cf85aee8a298df92b7ec466f47d9c8465 Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Thu, 16 Sep 2021 18:02:27 +0900 Subject: [PATCH 08/16] Fix coding style - Fixes header guards properly - Removes unnecessary header inclusion - Removes unused method Change-Id: Id7fcc4b985f094225ea1d3c45b0251a0eb9b8e4b Signed-off-by: Hwankyu Jhun --- src/ac-internal.cc | 6 ------ src/ac-internal.hh | 1 - src/message-sending-thread-internal.cc | 5 ----- src/message-sending-thread-internal.hh | 8 ++++---- 4 files changed, 4 insertions(+), 16 deletions(-) diff --git a/src/ac-internal.cc b/src/ac-internal.cc index 3f3af65..98addbb 100644 --- a/src/ac-internal.cc +++ b/src/ac-internal.cc @@ -93,12 +93,6 @@ int AccessController::Check(int fd, const std::string& sender_appid) { return ret; } -// LCOV_EXCL_START -int AccessController::SetCache(const std::string& sender) { - return -1; -} -// LCOV_EXCL_STOP - AccessController::Cynara::Cynara() : cynara_(nullptr, cynara_finish), client_(nullptr, std::free), user_(nullptr, std::free) { diff --git a/src/ac-internal.hh b/src/ac-internal.hh index 051a56d..7b60c62 100644 --- a/src/ac-internal.hh +++ b/src/ac-internal.hh @@ -53,7 +53,6 @@ class AccessController { std::unique_ptr user_; }; - int SetCache(const std::string& sender); int CheckTrusted(const std::string& sender_appid); int CheckPrivilege(const Cynara& c); diff --git a/src/message-sending-thread-internal.cc b/src/message-sending-thread-internal.cc index 470c1f5..e74c895 100644 --- a/src/message-sending-thread-internal.cc +++ b/src/message-sending-thread-internal.cc @@ -14,11 +14,6 @@ * limitations under the License. */ -#include -#include - -#include - #include "log-private.hh" #include "message-sending-thread-internal.hh" diff --git a/src/message-sending-thread-internal.hh b/src/message-sending-thread-internal.hh index b71b136..1d3acad 100644 --- a/src/message-sending-thread-internal.hh +++ b/src/message-sending-thread-internal.hh @@ -14,11 +14,11 @@ * limitations under the License. */ -#ifndef MESSAGE_SENDING_THREAD_HH_ -#define MESSAGE_SENDING_THREAD_HH_ +#ifndef MESSAGE_SENDING_THREAD_INTERNAL_HH_ +#define MESSAGE_SENDING_THREAD_INTERNAL_HH_ #include -#include +#include #include #include @@ -54,4 +54,4 @@ class MessageSendingThread { } // namespace internal } // namespace rpc_port -#endif // MESSAGE_SENDING_THREAD_HH_ +#endif // MESSAGE_SENDING_THREAD_INTERNAL_HH_ -- 2.7.4 From 9e75df5342da40dced240223076369a7101ab064 Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Thu, 16 Sep 2021 18:06:53 +0900 Subject: [PATCH 09/16] Fix locking the mutex This patch uses lock() and unlock() instead of std::lock_guard to reduce the instance creation of std::lock_guard. Change-Id: Ia7988caf0e9165bab79b785c19db7b989e41b077 Signed-off-by: Hwankyu Jhun --- src/port-internal.cc | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/port-internal.cc b/src/port-internal.cc index 77120f5..7e006a3 100644 --- a/src/port-internal.cc +++ b/src/port-internal.cc @@ -121,21 +121,18 @@ int Port::Read(void* buf, unsigned int size) { int max_timeout = MAX_CNT * MAX_SLEEP; /* 10 sec */ int fd; - { - std::lock_guard lock(mutex_); - fd = fd_; - if (fd < 0 || fd >= sysconf(_SC_OPEN_MAX)) { - _E("Invalid fd(%d)", fd); - return RPC_PORT_ERROR_IO_ERROR; - } + mutex_.lock(); + fd = fd_; + mutex_.unlock(); + if (fd < 0 || fd >= sysconf(_SC_OPEN_MAX)) { + _E("Invalid fd(%d)", fd); + return RPC_PORT_ERROR_IO_ERROR; } while (left) { - { - std::lock_guard lock(mutex_); - nb = read(fd, buffer, left); - } - + mutex_.lock(); + nb = read(fd_, buffer, left); + mutex_.unlock(); if (nb == 0) { _E("read_socket: ...read EOF, socket closed %d: nb %zd\n", fd, nb); return RPC_PORT_ERROR_IO_ERROR; -- 2.7.4 From b1b18e4f5fb93abfb905793a365f64a889782b89 Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Thu, 23 Sep 2021 11:54:12 +0900 Subject: [PATCH 10/16] Check debug socket existence If the socket path is not existed, rpc-port library doesn't try to connect to the debug-socket. Change-Id: Ia2c99e0c888e37764d31736d031d370cf288ea8c Signed-off-by: Hwankyu Jhun --- src/debug-port-internal.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/debug-port-internal.cc b/src/debug-port-internal.cc index 8e182fe..ddfd930 100644 --- a/src/debug-port-internal.cc +++ b/src/debug-port-internal.cc @@ -131,6 +131,9 @@ void DebugPort::Init() { } int DebugPort::Connect() { + if (access(PATH_RPC_PORT_UTIL_SOCK, F_OK) != 0) + return -1; + int fd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0); if (fd < 0) { _E("socket() is failed. errno(%d)", errno); -- 2.7.4 From b9c3d2eab91a58a327ffb251e42c10c5747b6622 Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Thu, 23 Sep 2021 11:02:57 +0900 Subject: [PATCH 11/16] Wait fd event to read data This patch uses poll() instead of nanosleep(). If the pol() returns a positive number, the Read() method tries to read the data from the file descriptor immediately. Change-Id: I4d0f3e548ba038538a2463b17a5dbff64e4cf8a2 Signed-off-by: Hwankyu Jhun --- src/port-internal.cc | 67 ++++++++++++++++++++++++++++++++++++++-------------- src/port-internal.hh | 1 + 2 files changed, 50 insertions(+), 18 deletions(-) diff --git a/src/port-internal.cc b/src/port-internal.cc index 7e006a3..c7c5726 100644 --- a/src/port-internal.cc +++ b/src/port-internal.cc @@ -23,6 +23,7 @@ #include #include +#include #include #include "include/rpc-port.h" @@ -30,15 +31,16 @@ #include "message-sending-thread-internal.hh" #include "port-internal.hh" -#define MAX_CNT 100 -#define MAX_SLEEP 100 -#define MIN_SLEEP 5 -#define BASE_SLEEP 1000 * 1000 -#define QUEUE_SIZE_MAX (1024 * 1024) /* 1MB */ -#define MAX_RETRY_CNT 10 - namespace rpc_port { namespace internal { +namespace { + +constexpr const int QUEUE_SIZE_MAX = 1024 * 1024; +constexpr const int MAX_RETRY_CNT = 10; +constexpr const int MAX_TIMEOUT = 1000; +constexpr const int MIN_TIMEOUT = 50; + +} // namespace Port::DelayMessage::DelayMessage(const char* msg, int index, int size) : message_(msg, msg + size), index_(index), size_(size) { @@ -115,10 +117,10 @@ int Port::UnsetPrivateSharing() { int Port::Read(void* buf, unsigned int size) { unsigned int left = size; ssize_t nb; - struct timespec TRY_SLEEP_TIME = { 0, MIN_SLEEP * BASE_SLEEP}; int bytes_read = 0; char* buffer = static_cast(buf); - int max_timeout = MAX_CNT * MAX_SLEEP; /* 10 sec */ + int max_timeout = MAX_TIMEOUT * MAX_RETRY_CNT; + int timeout = MIN_TIMEOUT; int fd; mutex_.lock(); @@ -138,16 +140,30 @@ int Port::Read(void* buf, unsigned int size) { return RPC_PORT_ERROR_IO_ERROR; } else if (nb == -1) { if (errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK) { - LOGI("read_socket: %d errno, sleep and retry ...", errno); - nanosleep(&TRY_SLEEP_TIME, 0); - max_timeout -= (TRY_SLEEP_TIME.tv_nsec / (BASE_SLEEP)); - if (max_timeout <= 0) { + LOGI("read_socket: %d errno, wait and retry ...", errno); + bool can_read = false; + while (!can_read && max_timeout > 0) { + auto start = std::chrono::steady_clock::now(); + mutex_.lock(); + can_read = CanRead(timeout); + mutex_.unlock(); + auto end = std::chrono::steady_clock::now(); + auto elapsed_time = + std::chrono::duration_cast( + end - start); + + max_timeout -= elapsed_time.count(); + + timeout *= 2; + if (timeout > MAX_TIMEOUT) + timeout = MAX_TIMEOUT; + } + + if (!can_read) { _E("read_socket: ...timed out fd %d: errno %d", fd, errno); return RPC_PORT_ERROR_IO_ERROR; } - TRY_SLEEP_TIME.tv_nsec *= 2; - if (TRY_SLEEP_TIME.tv_nsec > (MAX_SLEEP * BASE_SLEEP)) - TRY_SLEEP_TIME.tv_nsec = MAX_SLEEP * BASE_SLEEP; + continue; } @@ -158,19 +174,34 @@ int Port::Read(void* buf, unsigned int size) { left -= nb; buffer += nb; bytes_read += nb; - TRY_SLEEP_TIME.tv_nsec = MIN_SLEEP * BASE_SLEEP; + timeout = MIN_TIMEOUT; } return RPC_PORT_ERROR_NONE; } +bool Port::CanRead(int timeout) { + struct pollfd fds[1]; + fds[0].fd = fd_; + fds[0].events = POLLIN; + fds[0].revents = 0; + int ret = poll(fds, 1, timeout); + if (ret <= 0) { + _W("poll() is failed. fd(%d), error(%s)", + fd_, ret == 0 ? "timed out" : std::to_string(-errno).c_str()); + return false; + } + + return true; +} + bool Port::CanWrite() { struct pollfd fds[1]; fds[0].fd = fd_; fds[0].events = POLLOUT; fds[0].revents = 0; int ret = poll(fds, 1, 100); - if (ret == 0 || ret < 0) { + if (ret <= 0) { _W("poll() is failed. fd(%d), error(%s)", fd_, ret == 0 ? "timed out" : std::to_string(-errno).c_str()); return false; diff --git a/src/port-internal.hh b/src/port-internal.hh index ba8e66c..853b27f 100644 --- a/src/port-internal.hh +++ b/src/port-internal.hh @@ -67,6 +67,7 @@ class Port : public std::enable_shared_from_this { } private: + bool CanRead(int timeout); bool CanWrite(); void IgnoreIOEvent(); int ListenIOEvent(); -- 2.7.4 From 749bc1c1bf1c0f1afa6c84f18c8a09bc41399582 Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Thu, 23 Sep 2021 15:35:55 +0900 Subject: [PATCH 12/16] Release version 1.12.3 Changes: - Change file name - Fix coding style - Fix locking the mutex - Check debug socket existence - Wait fd event to read data Change-Id: I9f7f7baae0c2e9d6293b30826070d4b2755cdf2f Signed-off-by: Hwankyu Jhun --- packaging/rpc-port.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packaging/rpc-port.spec b/packaging/rpc-port.spec index 5cb0832..2a0289f 100644 --- a/packaging/rpc-port.spec +++ b/packaging/rpc-port.spec @@ -1,6 +1,6 @@ Name: rpc-port Summary: RPC Port library -Version: 1.12.2 +Version: 1.12.3 Release: 0 Group: Application Framework/Libraries License: Apache-2.0 -- 2.7.4 From 119f5380629a38e25f5c36027bb1a7ed950e9727 Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Mon, 27 Sep 2021 09:21:23 +0900 Subject: [PATCH 13/16] Remove unnecesary log print When EAGAIN error occurs, the rpc-port waits for POLLIN event. If the timeout error occurs, the rpc-port prints the error log. Change-Id: Icfaf163226072986e1d99e08a307ad2eb7ff83e0 Signed-off-by: Hwankyu Jhun --- src/port-internal.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/port-internal.cc b/src/port-internal.cc index c7c5726..aaccee8 100644 --- a/src/port-internal.cc +++ b/src/port-internal.cc @@ -140,7 +140,6 @@ int Port::Read(void* buf, unsigned int size) { return RPC_PORT_ERROR_IO_ERROR; } else if (nb == -1) { if (errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK) { - LOGI("read_socket: %d errno, wait and retry ...", errno); bool can_read = false; while (!can_read && max_timeout > 0) { auto start = std::chrono::steady_clock::now(); -- 2.7.4 From e8dd4984b7d2a86a70b997295453cce88be1d491 Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Mon, 27 Sep 2021 16:32:48 +0900 Subject: [PATCH 14/16] Fix Session Addition for DebugPort To reduce the error log print related to debug-port, calling AddSession() is moved. Change-Id: Iaf3f064d12054fa108d99ae796502043aeae801c Signed-off-by: Hwankyu Jhun --- src/proxy-internal.cc | 4 ++-- src/stub-internal.cc | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/proxy-internal.cc b/src/proxy-internal.cc index 22f99a8..91cb63e 100644 --- a/src/proxy-internal.cc +++ b/src/proxy-internal.cc @@ -264,8 +264,8 @@ int Proxy::ConnectSync(std::string appid, std::string port_name, main_port_.reset(new ProxyPort(this, fds_[0], target_appid_, false)); delegate_port_.reset(new ProxyPort(this, fds_[1], target_appid_)); - listener_->OnConnected(target_appid_, main_port_.get()); DebugPort::GetInst()->AddSession(port_name, target_appid_, fds_[0], fds_[1]); + listener_->OnConnected(target_appid_, main_port_.get()); return RPC_PORT_ERROR_NONE; } @@ -751,9 +751,9 @@ gboolean Proxy::Client::OnResponseReceived(GIOChannel* channel, proxy->fds_[1] = client_fd; proxy->delegate_port_.reset( new ProxyPort(proxy, proxy->fds_[1], proxy->target_appid_)); - listener->OnConnected(proxy->target_appid_, proxy->main_port_.get()); DebugPort::GetInst()->AddSession(proxy->port_name_, proxy->target_appid_, proxy->fds_[0], proxy->fds_[1]); + listener->OnConnected(proxy->target_appid_, proxy->main_port_.get()); _W("target_appid(%s), port_name(%s), main_fd(%d), delegate_fd(%d)", proxy->target_appid_.c_str(), proxy->port_name_.c_str(), proxy->fds_[0], proxy->fds_[1]); diff --git a/src/stub-internal.cc b/src/stub-internal.cc index 48fe4a3..f16035c 100644 --- a/src/stub-internal.cc +++ b/src/stub-internal.cc @@ -259,8 +259,8 @@ void Stub::AddAcceptedPort(const std::string& sender_appid, _W("sender_appid(%s), instance(%s), main_fd(%d), delegate_fd(%d)", sender_appid.c_str(), instance.c_str(), main_fd, fd); - listener_->OnConnected(sender_appid, instance); DebugPort::GetInst()->AddSession(port_name_, sender_appid, main_fd, fd); + listener_->OnConnected(sender_appid, instance); } std::recursive_mutex& Stub::GetMutex() const { -- 2.7.4 From c0e98c09f2d766596934f24e38138fa6e495fd85 Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Tue, 28 Sep 2021 14:31:45 +0900 Subject: [PATCH 15/16] Release version 1.12.4 Changes: - Remove unnecesary log print - Fix Session Addition for DebugPort Change-Id: Ic867209987e14104993350d07ce146cb9d051bc7 Signed-off-by: Hwankyu Jhun --- packaging/rpc-port.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packaging/rpc-port.spec b/packaging/rpc-port.spec index 2a0289f..d662e81 100644 --- a/packaging/rpc-port.spec +++ b/packaging/rpc-port.spec @@ -1,6 +1,6 @@ Name: rpc-port Summary: RPC Port library -Version: 1.12.3 +Version: 1.12.4 Release: 0 Group: Application Framework/Libraries License: Apache-2.0 -- 2.7.4 From e93c2abbfa54d7115abb46f4caf5689dac75db0f Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Thu, 7 Oct 2021 08:33:06 +0900 Subject: [PATCH 16/16] Use destructor attribute for disposing thread The rpc-port library can be loaded and unloaded using dlopen() and dlclose() in the runtime. In this case, the process has crashed by the thread of the rpc-port library. Before unloading the library, the thread has to be terminated. This patch uses destructor attribute to terminate the thread appropriately before unloading the library is finished. Change-Id: I2338a4419f02fb67dff042fd308ba1d590bf0ac3 Signed-off-by: Hwankyu Jhun --- src/debug-port-internal.cc | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/debug-port-internal.cc b/src/debug-port-internal.cc index ddfd930..2f3219e 100644 --- a/src/debug-port-internal.cc +++ b/src/debug-port-internal.cc @@ -25,19 +25,26 @@ #include "debug-port-internal.hh" #include "log-private.hh" +#undef RPC_DTOR +#define RPC_DTOR __attribute__ ((destructor)) + namespace rpc_port { namespace internal { namespace { const char PATH_RPC_PORT_UTIL_SOCK[] = "/run/aul/daemons/.rpc-port-util-sock"; + +RPC_DTOR void DebugPortDtor() { + DebugPort::GetInst()->Dispose(); +} + } // namespace std::atomic DebugPort::inst_; std::mutex DebugPort::mutex_; DebugPort::~DebugPort() { - if (!disposed_) - Dispose(); + Dispose(); } DebugPort* DebugPort::GetInst() { @@ -59,6 +66,10 @@ DebugPort* DebugPort::GetInst() { } void DebugPort::Dispose() { + std::lock_guard lock(GetMutex()); + if (disposed_) + return; + Unwatch(); JoinThread(); disposed_ = true; -- 2.7.4