From fe73b32dca63842ff81bbcecddfcf2a672f321a8 Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Mon, 19 Jul 2021 08:08:20 +0900 Subject: [PATCH 01/16] Fix static analysis issue - Initialize private members to nullptr Change-Id: I36ebc07223b157b207d2f7c4d4c61e21c21a9a92 Signed-off-by: Hwankyu Jhun --- test/unit_tests/rpc_port_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit_tests/rpc_port_test.cc b/test/unit_tests/rpc_port_test.cc index 78ac326..7163202 100644 --- a/test/unit_tests/rpc_port_test.cc +++ b/test/unit_tests/rpc_port_test.cc @@ -308,8 +308,8 @@ class RpcPortBase : public TestFixture { proxy_handle_ = nullptr; } - rpc_port_proxy_h proxy_handle_; - rpc_port_stub_h stub_handle_; + rpc_port_proxy_h proxy_handle_ = nullptr; + rpc_port_stub_h stub_handle_ = nullptr; bool touch_proxy_connected_event_cb_ = false; bool touch_stub_connected_event_cb_ = false; bool touch_proxy_rejected_event_cb_ = false; -- 2.7.4 From eb20db768fa2046c70ddf61b76781b7a1c0a19f7 Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Mon, 19 Jul 2021 08:15:55 +0900 Subject: [PATCH 02/16] Release version 1.9.2 Changes: - Fix static analysis issue Change-Id: I6949c16038588f2e951602aa68e219d43a69d646 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 80652e5..b02b731 100644 --- a/packaging/rpc-port.spec +++ b/packaging/rpc-port.spec @@ -1,6 +1,6 @@ Name: rpc-port Summary: RPC Port library -Version: 1.9.1 +Version: 1.9.2 Release: 0 Group: Application Framework/Libraries License: Apache-2.0 -- 2.7.4 From a3ad231fac16efbfe4603e063cc2e50ece245f42 Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Fri, 6 Aug 2021 10:43:14 +0900 Subject: [PATCH 03/16] Fix DebugPort Implementation If the instance is managed as a global variable, the process tries to access to the destroyed DebugPort instance while calling the exit handlers. Change-Id: Ia7cfa9d241266921221ecf28ce8b4b22dc4b95d6 Signed-off-by: Hwankyu Jhun --- src/debug-port-internal.cc | 21 +++++++++++++++++++++ src/debug-port-internal.hh | 16 ++++++---------- src/proxy-internal.cc | 10 +++++----- src/rpc-port.cc | 8 ++++---- src/stub-internal.cc | 4 ++-- 5 files changed, 38 insertions(+), 21 deletions(-) diff --git a/src/debug-port-internal.cc b/src/debug-port-internal.cc index 0d89d5f..8b28c77 100644 --- a/src/debug-port-internal.cc +++ b/src/debug-port-internal.cc @@ -34,11 +34,32 @@ const char RPC_PORT_SIGNAL_DEBUG[] = "Debug"; const char RPC_PORT_SIGNAL_NEW[] = "New"; } // namespace +std::atomic DebugPort::inst_; +std::mutex DebugPort::mutex_; + DebugPort::~DebugPort() { if (!disposed_) Dispose(); } +DebugPort* DebugPort::GetInst() { + DebugPort* inst = inst_.load(std::memory_order_acquire); + if (inst == nullptr) { + std::lock_guard lock(mutex_); + inst = inst_.load(std::memory_order_relaxed); + if (inst == nullptr) { + inst = new DebugPort(); + inst_.store(inst, std::memory_order_release); + } + } + + std::lock_guard rec_lock(inst->GetMutex()); + if (inst->disposed_) + inst->Init(); + + return inst; +} + void DebugPort::Dispose() { Unwatch(); Unsubscribe(); diff --git a/src/debug-port-internal.hh b/src/debug-port-internal.hh index 27eb66b..98a3042 100644 --- a/src/debug-port-internal.hh +++ b/src/debug-port-internal.hh @@ -74,14 +74,7 @@ class DebugPort { int delegate_port_; }; - static DebugPort& GetInst() { - static DebugPort inst; - - std::lock_guard lock(inst.GetMutex()); - if (inst.disposed_) - inst.Init(); - return inst; - } + static DebugPort* GetInst(); void Dispose(); bool IsConnected(); @@ -94,8 +87,11 @@ class DebugPort { const void* buf, unsigned int size); private: + static std::atomic inst_; + static std::mutex mutex_; + std::recursive_mutex& GetMutex() const { - return mutex_; + return rec_mutex_; } void Init(); @@ -134,7 +130,7 @@ class DebugPort { std::thread thread_; std::atomic is_running_; SharedQueue> queue_; - mutable std::recursive_mutex mutex_; + mutable std::recursive_mutex rec_mutex_; }; } // namespace internal diff --git a/src/proxy-internal.cc b/src/proxy-internal.cc index 9f7ac7c..c85f17a 100644 --- a/src/proxy-internal.cc +++ b/src/proxy-internal.cc @@ -262,14 +262,14 @@ 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]); + DebugPort::GetInst()->AddSession(port_name, target_appid_, fds_[0], fds_[1]); return RPC_PORT_ERROR_NONE; } void Proxy::DisconnectPort() { std::lock_guard lock(GetMutex()); if (main_port_.get() != nullptr) { - DebugPort::GetInst().RemoveSession(main_port_->GetFd()); + DebugPort::GetInst()->RemoveSession(main_port_->GetFd()); main_port_.reset(); } } @@ -456,7 +456,7 @@ gboolean Proxy::ProxyPort::OnSocketDisconnected(GIOChannel* channel, proxy->delegate_port_.reset(); proxy->listener_ = nullptr; listener->OnDisconnected(proxy->target_appid_); - DebugPort::GetInst().RemoveSession(fd); + DebugPort::GetInst()->RemoveSession(fd); return G_SOURCE_REMOVE;; } @@ -478,7 +478,7 @@ gboolean Proxy::ProxyPort::OnDataReceived(GIOChannel* channel, proxy->listener_ = nullptr; proxy->delegate_port_->SetSource(0); if (proxy->main_port_.get() != nullptr) { - DebugPort::GetInst().RemoveSession(proxy->main_port_->GetFd()); + DebugPort::GetInst()->RemoveSession(proxy->main_port_->GetFd()); proxy->main_port_.reset(); } proxy->delegate_port_.reset(); @@ -656,7 +656,7 @@ gboolean Proxy::Client::OnResponseReceived(GIOChannel* channel, 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_, + DebugPort::GetInst()->AddSession(proxy->port_name_, proxy->target_appid_, proxy->fds_[0], proxy->fds_[1]); _W("target_appid(%s), port_name(%s), main_fd(%d), delegate_fd(%d)", proxy->target_appid_.c_str(), proxy->port_name_.c_str(), diff --git a/src/rpc-port.cc b/src/rpc-port.cc index ca708d8..7340c2b 100644 --- a/src/rpc-port.cc +++ b/src/rpc-port.cc @@ -222,8 +222,8 @@ RPC_API int rpc_port_read(rpc_port_h h, void* buf, unsigned int size) { return ret; } - auto& debug_port = DebugPort::GetInst(); - debug_port.Send(port->GetFd(), true, seq, buf, size); + auto* debug_port = DebugPort::GetInst(); + debug_port->Send(port->GetFd(), true, seq, buf, size); return RPC_PORT_ERROR_NONE; } @@ -241,8 +241,8 @@ RPC_API int rpc_port_write(rpc_port_h h, const void* buf, unsigned int size) { if (ret < 0) return ret; - auto& debug_port = DebugPort::GetInst(); - debug_port.Send(port->GetFd(), false, seq, buf, size); + auto* debug_port = DebugPort::GetInst(); + debug_port->Send(port->GetFd(), false, seq, buf, size); return RPC_PORT_ERROR_NONE; } diff --git a/src/stub-internal.cc b/src/stub-internal.cc index 8a038c4..63cea83 100644 --- a/src/stub-internal.cc +++ b/src/stub-internal.cc @@ -154,7 +154,7 @@ void Stub::RemoveAcceptedPorts(std::string instance) { while (iter != ports_.end()) { if ((*iter)->GetInstance().compare(instance) == 0) { LOGI("Close: fd(%d)", (*iter)->GetFd()); - DebugPort::GetInst().RemoveSession((*iter)->GetFd()); + DebugPort::GetInst()->RemoveSession((*iter)->GetFd()); iter = ports_.erase(iter); } else { iter++; @@ -252,7 +252,7 @@ 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); + DebugPort::GetInst()->AddSession(port_name_, sender_appid, main_fd, fd); } std::recursive_mutex& Stub::GetMutex() const { -- 2.7.4 From 52792fc5b7631836a169680a78c7780c84f7906c Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Fri, 6 Aug 2021 12:10:57 +0900 Subject: [PATCH 04/16] Release version 1.9.3 Changes: - Fix DebugPort Implementation Change-Id: I98732667130139c3892762aa979960792964098d 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 b02b731..5627f21 100644 --- a/packaging/rpc-port.spec +++ b/packaging/rpc-port.spec @@ -1,6 +1,6 @@ Name: rpc-port Summary: RPC Port library -Version: 1.9.2 +Version: 1.9.3 Release: 0 Group: Application Framework/Libraries License: Apache-2.0 -- 2.7.4 From 696a464a2b28909a8ec6e40c66f936e907558685 Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Thu, 12 Aug 2021 15:06:38 +0900 Subject: [PATCH 05/16] Fix deadlock issue When the rpc_port_proxy_connect() is called in the sub thread, the port appeared event can be received in the main thread. In this case, the process can be a deadlock state as below: +------------------------------------------------------------------------------+ | -- #0 0xb5d3e6c8 in __lll_lock_wait () from /usr/lib/libpthread-2.30.so | | -- #1 0xb5d35de4 in __pthread_mutex_lock () from /usr/lib/libpthread-2.30.so| | -- #2 0xa63ee315 in _ZN8rpc_port8internal5Proxy14OnPortAppearedEPKcS3_iPv ()| | from /usr/lib/librpc-port.so.1.9.3 | | -- #3 0xb5f69721 in _ZN12_GLOBAL__N_19WatchInfo8AppComCbEPKc20aul_app_com_re| | sult_eP9_bundle_tPv () from /usr/lib/libaul.so.0.38.0 | | -- #4 0xb5f74d2d in app_com_recv () from /usr/lib/libaul.so.0.38.0 | | -- #5 0xb5f74fe7 in __dispatch_request () from /usr/lib/libaul.so.0.38.0 | | -- #6 0xb5b10fb3 in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.| 0.6200.3 | | -- #7 0xb58a01a9 in _ecore_glib_select.lto_priv.0 () from /usr/lib/libecore.| | so.1.25.1 | | -- #8 0xb589f037 in _ecore_main_select () from /usr/lib/libecore.so.1.25.1 | | -- #9 0xb589f8c7 in _ecore_main_loop_iterate_internal () from /usr/lib/libec| | ore.so.1.25.1 | | -- [pthread wait detected] pthread_mutex_lock/pthread_join owner tid:1412 | +------------------------------------------------------------------------------+ | -- [User Backtrace] [Thread state:S (sleeping), Frozen:No] | | -- Pid: 683, Tid: 1412, comm: Parallel[0] exec_start[69.429195162] | | -- #0 0xb5d3e6c8 in __lll_lock_wait () from /usr/lib/libpthread-2.30.so | | -- #1 0xb5d35de4 in __pthread_mutex_lock () from /usr/lib/libpthread-2.30.so| | -- #2 0xb5f789b5 in aul_app_com_leave () from /usr/lib/libaul.so.0.38.0 | | -- #3 0xb5f6d197 in aul_rpc_port_remove_watch () from /usr/lib/libaul.so.0.3| | 8.0 | | -- #4 0xa63ec503 in _ZN8rpc_port8internal5Proxy6CancelEv () from /usr/lib/li| | brpc-port.so.1.9.3 | | -- #5 0xa63ee329 in _ZN8rpc_port8internal5Proxy14OnPortAppearedEPKcS3_iPv ()| | from /usr/lib/librpc-port.so.1.9.3 | | -- #6 0xa63ee45b in _ZN8rpc_port8internal5Proxy5WatchEv () from /usr/lib/lib| | rpc-port.so.1.9.3 | | -- #7 0xa63ee58f in _ZN8rpc_port8internal5Proxy7ConnectENSt7__cxx1112basic_s| | tringIcSt11char_traitsIcESaIcEEES7_PNS1_14IEventListener| | E () from /usr/lib/librpc-port.so.1.9.3 | | -- #8 0xa63efd51 in rpc_port_proxy_connect () from /usr/lib/librpc-port.so.1| | .9.3 | +------------------------------------------------------------------------------+ This patch fixes the proxy handle management using the shared_ptr. And, the port check is moved to the main thread to avoid the deadlock issue. Change-Id: I8ef7f775491427e48eb5894c66dc1629d172a4d5 Signed-off-by: Hwankyu Jhun --- src/proxy-internal.cc | 137 ++++++++++++++++++++++++++++++++++++++++++++------ src/proxy-internal.hh | 12 ++++- src/rpc-port.cc | 88 ++++++++++++++++++-------------- 3 files changed, 182 insertions(+), 55 deletions(-) diff --git a/src/proxy-internal.cc b/src/proxy-internal.cc index c85f17a..f22c6ca 100644 --- a/src/proxy-internal.cc +++ b/src/proxy-internal.cc @@ -104,6 +104,7 @@ Proxy::~Proxy() { std::lock_guard lock(GetMutex()); _D("Proxy::~Proxy()"); listener_ = nullptr; + UnsetIdler(); UnsetConnTimer(); Cancel(); } @@ -284,13 +285,8 @@ int Proxy::Watch() { return -1; } - if (Aul::ExistPort(real_appid_, port_name_, rpc_port_get_target_uid())) { - OnPortAppeared(real_appid_.c_str(), port_name_.c_str(), -1, this); - } else { - OnPortVanished(real_appid_.c_str(), port_name_.c_str(), -1, this); - SetConnTimer(); - } - + SetConnTimer(); + SetIdler(); return 0; } @@ -324,15 +320,59 @@ std::recursive_mutex& Proxy::GetMutex() const { } void Proxy::SetConnTimer() { - if (conn_timer_ == 0) - conn_timer_ = g_timeout_add_seconds(10, OnTimedOut, this); + if (conn_timer_data_) { + _W("Already exists"); + return; + } + + conn_timer_data_ = CreateWeakPtr(); + if (conn_timer_data_ == nullptr) { + _E("Out of memory"); + return; + } + + g_timeout_add_seconds(10, OnTimedOut, conn_timer_data_); } void Proxy::UnsetConnTimer() { - if (conn_timer_) { - g_source_remove(conn_timer_); - conn_timer_ = 0; + if (conn_timer_data_ == nullptr) + return; + + GSource* source = g_main_context_find_source_by_user_data(nullptr, + conn_timer_data_); + if (source && !g_source_is_destroyed(source)) + g_source_destroy(source); + + DestroyWeakPtr(conn_timer_data_); + conn_timer_data_ = nullptr; +} + +void Proxy::SetIdler() { + if (idler_data_) { + _W("Already exists"); + return; } + + idler_data_ = CreateWeakPtr(); + if (idler_data_ == nullptr) { + _E("Out of memory"); + return; + } + + g_idle_add(OnIdle, idler_data_); +} + +void Proxy::UnsetIdler() { + if (idler_data_ == nullptr) + return; + + GSource* source = g_main_context_find_source_by_user_data(nullptr, + idler_data_); + if (source && !g_source_is_destroyed(source)) + g_source_destroy(source); + + DestroyWeakPtr(idler_data_); + idler_data_ = nullptr; } void Proxy::OnPortAppeared(const char* app_id, const char* port_name, int pid, @@ -340,6 +380,13 @@ void Proxy::OnPortAppeared(const char* app_id, const char* port_name, int pid, _W("app_id(%s), port_name(%s), pid(%d)", app_id, port_name, pid); auto* proxy = static_cast(user_data); std::lock_guard lock(proxy->GetMutex()); + proxy->UnsetIdler(); + + if (proxy->watch_handle_ == nullptr) { + _E("Invalid context"); + return; + } + auto* listener = proxy->listener_; if (listener == nullptr) { _E("Invalid context"); @@ -362,21 +409,67 @@ void Proxy::OnPortAppeared(const char* app_id, const char* port_name, int pid, void Proxy::OnPortVanished(const char* app_id, const char* port_name, int pid, void* user_data) { _W("app_id(%s), port_name(%s), pid(%d)", app_id, port_name, pid); + auto* proxy = static_cast(user_data); + std::lock_guard lock(proxy->GetMutex()); + proxy->UnsetIdler(); } gboolean Proxy::OnTimedOut(gpointer user_data) { _E("Timed out"); - auto* proxy = static_cast(user_data); + auto* ptr = static_cast*>(user_data); + auto proxy = ptr->lock(); + if (proxy == nullptr) { + _E("Proxy is nullptr"); + return G_SOURCE_REMOVE; + } + std::lock_guard lock(proxy->GetMutex()); + if (proxy->conn_timer_data_ == nullptr) { + _E("Invalid context. proxy(%p)", proxy.get()); + return G_SOURCE_REMOVE; + } + + DestroyWeakPtr(proxy->conn_timer_data_); + proxy->conn_timer_data_ = nullptr; + auto* listener = proxy->listener_; if (listener == nullptr) { _E("Invalid context"); return G_SOURCE_REMOVE; } - proxy->UnsetConnTimer(); listener->OnRejected(proxy->target_appid_, RPC_PORT_ERROR_IO_ERROR); - return G_SOURCE_CONTINUE; + return G_SOURCE_REMOVE; +} + +gboolean Proxy::OnIdle(gpointer user_data) { + auto* ptr = static_cast*>(user_data); + auto proxy = ptr->lock(); + if (proxy == nullptr) { + _E("Proxy is nullptr"); + return G_SOURCE_REMOVE; + } + + std::lock_guard lock(proxy->GetMutex()); + if (proxy->idler_data_ == nullptr) { + _E("Invalid context. proxy(%p)", proxy.get()); + return G_SOURCE_REMOVE; + } + + DestroyWeakPtr(proxy->idler_data_); + proxy->idler_data_ = nullptr; + + bool exist = Aul::ExistPort(proxy->real_appid_, proxy->port_name_, + rpc_port_get_target_uid()); + if (exist) { + proxy->OnPortAppeared(proxy->real_appid_.c_str(), + proxy->port_name_.c_str(), -1, proxy.get()); + } else { + proxy->OnPortVanished(proxy->real_appid_.c_str(), + proxy->port_name_.c_str(), -1, proxy.get()); + } + + return G_SOURCE_REMOVE; } Proxy::ProxyPort::ProxyPort(Proxy* parent, int fd, const std::string& id, @@ -670,5 +763,19 @@ gboolean Proxy::Client::OnResponseReceived(GIOChannel* channel, return G_SOURCE_REMOVE; } +std::shared_ptr Proxy::GetSharedPtr() { + return shared_from_this(); +} + +gpointer Proxy::CreateWeakPtr() { + auto* ptr = new (std::nothrow) std::weak_ptr(GetSharedPtr()); + return static_cast(ptr); +} + +void Proxy::DestroyWeakPtr(gpointer data) { + auto* ptr = static_cast*>(data); + delete ptr; +} + } // namespace internal } // namespace rpc_port diff --git a/src/proxy-internal.hh b/src/proxy-internal.hh index df0fbcb..414888f 100644 --- a/src/proxy-internal.hh +++ b/src/proxy-internal.hh @@ -32,7 +32,7 @@ namespace rpc_port { namespace internal { -class Proxy { +class Proxy : public std::enable_shared_from_this { public: Proxy(); virtual ~Proxy(); @@ -115,6 +115,7 @@ class Proxy { static void OnPortVanished(const char* app_id, const char* port_name, int pid, void* user_data); static gboolean OnTimedOut(gpointer user_data); + static gboolean OnIdle(gpointer user_data); void SetRealAppId(const std::string& alias_appid); std::recursive_mutex& GetMutex() const; @@ -123,6 +124,12 @@ class Proxy { void Cancel(); void SetConnTimer(); void UnsetConnTimer(); + void SetIdler(); + void UnsetIdler(); + + std::shared_ptr GetSharedPtr(); + gpointer CreateWeakPtr(); + static void DestroyWeakPtr(gpointer data); private: std::string port_name_; @@ -135,7 +142,8 @@ class Proxy { std::unique_ptr main_client_; std::unique_ptr delegate_client_; aul_rpc_port_watch_h watch_handle_ = nullptr; - guint conn_timer_ = 0; + gpointer conn_timer_data_ = nullptr; + gpointer idler_data_ = nullptr; mutable std::recursive_mutex mutex_; }; diff --git a/src/rpc-port.cc b/src/rpc-port.cc index 7340c2b..1410a04 100644 --- a/src/rpc-port.cc +++ b/src/rpc-port.cc @@ -250,9 +250,19 @@ RPC_API int rpc_port_proxy_create(rpc_port_proxy_h* h) { if (h == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - auto p = new ::ProxyExt(); + auto p = new (std::nothrow) std::shared_ptr<::ProxyExt>( + new (std::nothrow) ::ProxyExt()); + if (p == nullptr) { + _E("Out of memory"); + } else if (p->get() == nullptr) { + _E("Out of memory"); + delete p; + p = nullptr; + } else { + _W("rpc_port_proxy_create(%p)", p->get()); + } + *h = p; - _W("rpc_port_proxy_create(%p)", p); return RPC_PORT_ERROR_NONE; } @@ -260,15 +270,16 @@ RPC_API int rpc_port_proxy_destroy(rpc_port_proxy_h h) { if (h == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - auto p = static_cast<::ProxyExt*>(h); - _W("rpc_port_proxy_destroy(%p)", p); - p->SetDestroying(true); - p->DisconnectPort(); + auto p = static_cast*>(h); + auto* proxy = p->get(); + _W("rpc_port_proxy_destroy(%p)", proxy); + proxy->SetDestroying(true); + proxy->DisconnectPort(); g_idle_add_full(G_PRIORITY_HIGH, [](gpointer data) -> gboolean { - auto p = static_cast<::ProxyExt*>(data); - _W("rpc_port_proxy_destroy(%p)", p); + auto p = static_cast*>(data); + _W("rpc_port_proxy_destroy(%p)", p->get()); delete p; return G_SOURCE_REMOVE; }, h, nullptr); @@ -280,11 +291,11 @@ RPC_API int rpc_port_proxy_connect(rpc_port_proxy_h h, const char* appid, if (h == nullptr || appid == nullptr || port == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - _W("rpc_port_proxy_connect(%p, %s, %s)", h, appid, port); - auto p = static_cast<::ProxyExt*>(h); - std::lock_guard lock(p->GetMutex()); - - return p->Connect(appid, port, p); + auto p = static_cast*>(h); + auto* proxy = p->get(); + _W("rpc_port_proxy_connect(%p, %s, %s)", proxy, appid, port); + std::lock_guard lock(proxy->GetMutex()); + return proxy->Connect(appid, port, proxy); } // LCOV_EXCL_START @@ -293,11 +304,11 @@ RPC_API int rpc_port_proxy_connect_sync(rpc_port_proxy_h h, const char* appid, if (h == nullptr || appid == nullptr || port == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - _W("rpc_port_proxy_connect(%p, %s, %s)", h, appid, port); - auto p = static_cast<::ProxyExt*>(h); - std::lock_guard lock(p->GetMutex()); - - return p->ConnectSync(appid, port, p); + auto p = static_cast*>(h); + auto* proxy = p->get(); + _W("rpc_port_proxy_connect(%p, %s, %s)", proxy, appid, port); + std::lock_guard lock(proxy->GetMutex()); + return proxy->ConnectSync(appid, port, proxy); } // LCOV_EXCL_STOP @@ -306,10 +317,10 @@ RPC_API int rpc_port_proxy_add_connected_event_cb(rpc_port_proxy_h h, if (h == nullptr || cb == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - auto p = static_cast<::ProxyExt*>(h); - std::lock_guard lock(p->GetMutex()); - - p->AddConnectedEventListener(cb, user_data); + auto p = static_cast*>(h); + auto* proxy = p->get(); + std::lock_guard lock(proxy->GetMutex()); + proxy->AddConnectedEventListener(cb, user_data); return RPC_PORT_ERROR_NONE; } @@ -318,10 +329,10 @@ RPC_API int rpc_port_proxy_add_disconnected_event_cb(rpc_port_proxy_h h, if (h == nullptr || cb == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - auto p = static_cast<::ProxyExt*>(h); - std::lock_guard lock(p->GetMutex()); - - p->AddDisconnectedEventListener(cb, user_data); + auto p = static_cast*>(h); + auto* proxy = p->get(); + std::lock_guard lock(proxy->GetMutex()); + proxy->AddDisconnectedEventListener(cb, user_data); return RPC_PORT_ERROR_NONE; } @@ -330,10 +341,10 @@ RPC_API int rpc_port_proxy_add_rejected_event_cb(rpc_port_proxy_h h, if (h == nullptr || cb == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - auto p = static_cast<::ProxyExt*>(h); - std::lock_guard lock(p->GetMutex()); - - p->AddRejectedEventListener(cb, user_data); + auto p = static_cast*>(h); + auto* proxy = p->get(); + std::lock_guard lock(proxy->GetMutex()); + proxy->AddRejectedEventListener(cb, user_data); return RPC_PORT_ERROR_NONE; } @@ -342,10 +353,10 @@ RPC_API int rpc_port_proxy_add_received_event_cb(rpc_port_proxy_h h, if (h == nullptr || cb == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - auto p = static_cast<::ProxyExt*>(h); - std::lock_guard lock(p->GetMutex()); - - p->AddReceivedEventListener(cb, user_data); + auto p = static_cast*>(h); + auto* proxy = p->get(); + std::lock_guard lock(proxy->GetMutex()); + proxy->AddReceivedEventListener(cb, user_data); return RPC_PORT_ERROR_NONE; } @@ -354,19 +365,20 @@ RPC_API int rpc_port_proxy_get_port(rpc_port_proxy_h h, if (h == nullptr || port == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - auto p = static_cast<::ProxyExt*>(h); - std::lock_guard lock(p->GetMutex()); + auto p = static_cast*>(h); + auto* proxy = p->get(); + std::lock_guard lock(proxy->GetMutex()); rpc_port_h ret_port; switch (type) { case RPC_PORT_PORT_MAIN: - ret_port = static_cast(p->GetPort().get()); + ret_port = static_cast(proxy->GetPort().get()); if (ret_port == nullptr) return RPC_PORT_ERROR_IO_ERROR; *port = ret_port; break; case RPC_PORT_PORT_CALLBACK: - ret_port = static_cast(p->GetDelegatePort().get()); + ret_port = static_cast(proxy->GetDelegatePort().get()); if (ret_port == nullptr) return RPC_PORT_ERROR_IO_ERROR; *port = ret_port; -- 2.7.4 From 9ddf51c68c268fdfa5cc8d15f4d4dd94b30893bb Mon Sep 17 00:00:00 2001 From: Changgyu Choi Date: Thu, 12 Aug 2021 10:03:16 +0900 Subject: [PATCH 06/16] Fix coding style Change-Id: I77493692e72af5b2d4af5ca1113ef3f04f608de5 Signed-off-by: Changgyu Choi --- src/ac-internal.cc | 2 ++ src/client-socket-internal.hh | 2 +- src/debug-port-internal.cc | 2 ++ src/debug-port-internal.hh | 1 + src/exception-internal.cc | 2 ++ src/port-internal.cc | 4 +++- src/port-internal.hh | 6 +++--- src/proxy-internal.cc | 2 ++ src/proxy-internal.hh | 2 +- src/request-internal.cc | 2 ++ src/response-internal.hh | 2 +- src/rpc-port.cc | 3 ++- src/server-socket-internal.hh | 2 +- src/stub-internal.cc | 5 ++++- utils/debug-port.cc | 1 + utils/logger.cc | 3 +-- utils/message.cc | 1 + 17 files changed, 30 insertions(+), 12 deletions(-) diff --git a/src/ac-internal.cc b/src/ac-internal.cc index a0b5e7a..3f3af65 100644 --- a/src/ac-internal.cc +++ b/src/ac-internal.cc @@ -20,6 +20,8 @@ #include #include +#include + #include "ac-internal.hh" #include "aul-internal.hh" #include "log-private.hh" diff --git a/src/client-socket-internal.hh b/src/client-socket-internal.hh index de7e960..2446e38 100644 --- a/src/client-socket-internal.hh +++ b/src/client-socket-internal.hh @@ -25,7 +25,7 @@ namespace internal { class ClientSocket { public: ClientSocket(); - ClientSocket(int fd); + explicit ClientSocket(int fd); virtual ~ClientSocket(); void Close(); diff --git a/src/debug-port-internal.cc b/src/debug-port-internal.cc index 8b28c77..ab660a3 100644 --- a/src/debug-port-internal.cc +++ b/src/debug-port-internal.cc @@ -20,6 +20,8 @@ #include #include +#include + #include "debug-port-internal.hh" #include "log-private.hh" diff --git a/src/debug-port-internal.hh b/src/debug-port-internal.hh index 98a3042..ac63f41 100644 --- a/src/debug-port-internal.hh +++ b/src/debug-port-internal.hh @@ -27,6 +27,7 @@ #include #include #include +#include #include "port-internal.hh" #include "shared-queue-internal.hh" diff --git a/src/exception-internal.cc b/src/exception-internal.cc index 0ee27f0..8e21f20 100644 --- a/src/exception-internal.cc +++ b/src/exception-internal.cc @@ -16,6 +16,8 @@ #include "exception-internal.hh" +#include + namespace rpc_port { namespace internal { diff --git a/src/port-internal.cc b/src/port-internal.cc index 79438f2..9ae6802 100644 --- a/src/port-internal.cc +++ b/src/port-internal.cc @@ -24,6 +24,8 @@ #include #include +#include + #include "include/rpc-port.h" #include "log-private.hh" #include "port-internal.hh" @@ -268,7 +270,7 @@ gboolean Port::OnEventReceived(gint fd, GIOCondition cond, void Port::ClearQueue() { std::lock_guard lock(mutex_); - while(queue_.empty() == false) + while (queue_.empty() == false) queue_.pop(); if (delay_src_id_ != 0) { diff --git a/src/port-internal.hh b/src/port-internal.hh index d6fc428..bda4d41 100644 --- a/src/port-internal.hh +++ b/src/port-internal.hh @@ -84,9 +84,9 @@ class Port { }; enum PortStatus { - PORT_STATUS_ERROR_NONE, - PORT_STATUS_ERROR_IO_ERROR, - PORT_STATUS_ERROR_RESOURCE_UNAVAILABLE + PORT_STATUS_ERROR_NONE, + PORT_STATUS_ERROR_IO_ERROR, + PORT_STATUS_ERROR_RESOURCE_UNAVAILABLE }; int PushDelayedMessage(std::shared_ptr dm); diff --git a/src/proxy-internal.cc b/src/proxy-internal.cc index f22c6ca..35abc75 100644 --- a/src/proxy-internal.cc +++ b/src/proxy-internal.cc @@ -21,6 +21,8 @@ #include #include +#include +#include #include "aul-internal.hh" #include "debug-port-internal.hh" diff --git a/src/proxy-internal.hh b/src/proxy-internal.hh index 414888f..5003539 100644 --- a/src/proxy-internal.hh +++ b/src/proxy-internal.hh @@ -87,7 +87,7 @@ class Proxy : public std::enable_shared_from_this { class Client : public ClientSocket { public: - Client(Proxy* parent); + explicit Client(Proxy* parent); virtual ~Client(); static Client* Create(Proxy* parent, const std::string& endpoint); diff --git a/src/request-internal.cc b/src/request-internal.cc index ef15ad5..af78026 100644 --- a/src/request-internal.cc +++ b/src/request-internal.cc @@ -16,6 +16,8 @@ #include "request-internal.hh" +#include + namespace rpc_port { namespace internal { diff --git a/src/response-internal.hh b/src/response-internal.hh index 4855b81..25227d3 100644 --- a/src/response-internal.hh +++ b/src/response-internal.hh @@ -25,7 +25,7 @@ namespace internal { class Response : public tizen_base::Parcelable { public: - Response(int result); + explicit Response(int result); Response(); ~Response(); diff --git a/src/rpc-port.cc b/src/rpc-port.cc index 1410a04..310cfbc 100644 --- a/src/rpc-port.cc +++ b/src/rpc-port.cc @@ -21,6 +21,7 @@ #include #include #include +#include #include "include/rpc-port-internal.h" #include "include/rpc-port.h" @@ -47,7 +48,7 @@ class Event { class ProxyExt : public Proxy, public Proxy::IEventListener { public: - explicit ProxyExt() : Proxy(), destroying_(false) {} + ProxyExt() : Proxy(), destroying_(false) {} virtual ~ProxyExt() = default; void AddConnectedEventListener(rpc_port_proxy_connected_event_cb cb, diff --git a/src/server-socket-internal.hh b/src/server-socket-internal.hh index ae025ac..47ca970 100644 --- a/src/server-socket-internal.hh +++ b/src/server-socket-internal.hh @@ -24,7 +24,7 @@ namespace internal { class ServerSocket { public: - ServerSocket(int fd); + explicit ServerSocket(int fd); virtual ~ServerSocket(); bool IsClosed(); diff --git a/src/stub-internal.cc b/src/stub-internal.cc index 63cea83..553a9a6 100644 --- a/src/stub-internal.cc +++ b/src/stub-internal.cc @@ -20,6 +20,9 @@ #include #include +#include +#include + #include "aul-internal.hh" #include "debug-port-internal.hh" #include "include/rpc-port.h" @@ -222,7 +225,7 @@ gboolean Stub::OnSocketDisconnected(GIOChannel* channel, GIOCondition cond, } } - return G_SOURCE_REMOVE;; + return G_SOURCE_REMOVE; } void Stub::AddAcceptedPort(const std::string& sender_appid, diff --git a/utils/debug-port.cc b/utils/debug-port.cc index 7a5229b..202d9da 100644 --- a/utils/debug-port.cc +++ b/utils/debug-port.cc @@ -22,6 +22,7 @@ #include #include +#include #include #include "debug-port.hh" diff --git a/utils/logger.cc b/utils/logger.cc index 7bb1895..8d1608f 100644 --- a/utils/logger.cc +++ b/utils/logger.cc @@ -87,8 +87,7 @@ int Logger::Read(void* buf, unsigned int size) { return 0; } -rpc_port_parcel_h Logger::Read() -{ +rpc_port_parcel_h Logger::Read() { int size = 0; int ret = Read(reinterpret_cast(&size), sizeof(size)); if (ret < 0 || size <= 0) diff --git a/utils/message.cc b/utils/message.cc index 17fa647..47be341 100644 --- a/utils/message.cc +++ b/utils/message.cc @@ -20,6 +20,7 @@ #include #include #include +#include #include "message.hh" -- 2.7.4 From 277701b3ffe1e210e385709025c07d82ed878948 Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Fri, 13 Aug 2021 11:06:47 +0900 Subject: [PATCH 07/16] Release version 1.9.4 Changes: - Fix deadlock issue - Fix coding style Change-Id: Id585a76b59363c92c380480bd82302cbc57efbb6 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 5627f21..b2ee437 100644 --- a/packaging/rpc-port.spec +++ b/packaging/rpc-port.spec @@ -1,6 +1,6 @@ Name: rpc-port Summary: RPC Port library -Version: 1.9.3 +Version: 1.9.4 Release: 0 Group: Application Framework/Libraries License: Apache-2.0 -- 2.7.4 From 6cac26ae24036dc790cf46314063afbbc3ab0374 Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Fri, 21 May 2021 13:06:27 +0900 Subject: [PATCH 08/16] Add the header handle of the parcel Adds: - rpc_port_parcel_get_header() - rpc_port_parcel_header_set_seq_num() - rpc_port_parcel_header_get_seq_num() - rpc_port_parcel_header_get_timestamp() Change-Id: I4049532b2c6a92e657861dbfc1dbe66a895081d1 Signed-off-by: Hwankyu Jhun --- include/rpc-port-internal.h | 4 +- include/rpc-port-parcel.h | 65 +++++++++- src/parcel-header-internal.cc | 60 +++++++++ src/parcel-header-internal.hh | 48 ++++++++ src/parcel-internal.cc | 71 +++++++++++ src/parcel-internal.hh | 48 ++++++++ src/rpc-port-parcel.cc | 212 +++++++++++++++++++------------- test/unit_tests/rpc_port_parcel_test.cc | 82 ++++++++++++ 8 files changed, 501 insertions(+), 89 deletions(-) create mode 100644 src/parcel-header-internal.cc create mode 100644 src/parcel-header-internal.hh create mode 100644 src/parcel-internal.cc create mode 100644 src/parcel-internal.hh diff --git a/include/rpc-port-internal.h b/include/rpc-port-internal.h index ab99261..999f73f 100644 --- a/include/rpc-port-internal.h +++ b/include/rpc-port-internal.h @@ -51,7 +51,7 @@ uid_t rpc_port_get_target_uid(void); * @param[in] extra The extra data * @return @c 0 on success, * otherwise a negative error value - * @see rpc_port_register_proc_info() + * @see rpc_port_deregister_proc_info() */ int rpc_port_register_proc_info(const char *proc_name, bundle *extra); @@ -60,7 +60,7 @@ int rpc_port_register_proc_info(const char *proc_name, bundle *extra); * @since_tizen 6.5 * @return @c 0 on success, * otherwise a negative error value - * @see rpc_port_deregister_proc_info() + * @see rpc_port_register_proc_info() */ int rpc_port_deregister_proc_info(void); diff --git a/include/rpc-port-parcel.h b/include/rpc-port-parcel.h index 3b39802..292dfd6 100644 --- a/include/rpc-port-parcel.h +++ b/include/rpc-port-parcel.h @@ -18,6 +18,8 @@ #define __TIZEN_APPFW_RPC_PORT_PARCEL_INCLUDE_H__ #include +#include + #include #include @@ -37,12 +39,18 @@ extern "C" { typedef void *rpc_port_parcel_h; /** + * @brief The header handle of the rpc port parcel. + * @since_tizen 6.5 + */ +typedef void *rpc_port_parcel_header_h; + +/** * @brief The interface for converting data to/from a parcel. * @since_tizen @if MOBILE 4.0 @elseif WEARABLE 5.0 @endif */ typedef struct __rpc_port_parcelable { - void (*to)(rpc_port_parcel_h h, void *data); - void (*from)(rpc_port_parcel_h h, void *data); + void (*to)(rpc_port_parcel_h h, void *data); /**< The function pointer to read from parcel */ + void (*from)(rpc_port_parcel_h h, void *data); /**< The function pointer to write to parcel */ } rpc_port_parcelable_t; /** @@ -424,6 +432,59 @@ int rpc_port_parcel_burst_read(rpc_port_parcel_h h, unsigned char *buf, unsigned int rpc_port_parcel_burst_write(rpc_port_parcel_h h, const unsigned char *buf, unsigned int size); /** + * @brief Gets the header handle of the rpc port parcel. + * @since_tizen 6.5 + * @remarks The @a header is managed by the platform and will be released when rpc_port_parcel_destroy() is called. + * @param[in] h The rpc port parcel handle + * @param[out] header The header handle of the rpc port parcel + * @return @c 0 on success, + * otherwise a negative error value + * @retval #RPC_PORT_ERROR_NONE Successful + * @retval #RPC_PORT_ERROR_INVALID_PARAMETER Invalid parameter + */ +int rpc_port_parcel_get_header(rpc_port_parcel_h h, rpc_port_parcel_header_h *header); + +/** + * @brief Sets the sequence number to the header handle of the rpc port parcel. + * @since_tizen 6.5 + * @param[in] header The header handler of the rpc port parcel + * @param[in] seq_num The sequence number + * @return @c 0 on success, + * otherwise a negative error value + * @retval #RPC_PORT_ERROR_NONE Successful + * @retval #RPC_PORT_ERROR_INVALID_PARAMETER Invalid parameter + * @see rpc_port_parcel_header_get_seq_num() + */ +int rpc_port_parcel_header_set_seq_num(rpc_port_parcel_header_h header, int seq_num); + +/** + * @brief Gets the sequence number from the header handle of the rpc port parcel. + * @since_tizen 6.5 + * @param[in] header The header handle of the rpc port parcel + * @param[out] seq_num The sequence number + * @return @c 0 on success, + * otherwise a negative error value + * @retval #RPC_PORT_ERROR_NONE Successful + * @retval #RPC_PORT_ERROR_INVALID_PARAMETER Invalid parameter + * @see rpc_port_parcel_header_set_seq_num() + */ +int rpc_port_parcel_header_get_seq_num(rpc_port_parcel_header_h header, int *seq_num); + +/** + * @brief Gets the timestamp from the header handle of the rpc port parcel. + * @since_tizen 6.5 + * @remarks The @a timestamp represents monotonic time since some unspecified starting point. + * To get elapsed time properly, you have to get the timestamp using the clock_gettime() with CLOCK_MONITONIC_RAW. + * @param[in] header The header handle of the rpc port parcel + * @param[out] timestamp The timestamp + * @return @c 0 on success, + * otherwise a negative error value + * @retval #RPC_PORT_ERROR_NONE Successful + * @retval #RPC_PORT_ERROR_INVALID_PARAMETER Invalid parameter + */ +int rpc_port_parcel_header_get_timestamp(rpc_port_parcel_header_h header, struct timespec *timestamp); + +/** * @} */ diff --git a/src/parcel-header-internal.cc b/src/parcel-header-internal.cc new file mode 100644 index 0000000..c1d51fb --- /dev/null +++ b/src/parcel-header-internal.cc @@ -0,0 +1,60 @@ +/* + * Copyright (c) 2021 Samsung Electronics Co., Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include "parcel-header-internal.hh" + +namespace rpc_port { +namespace internal { + +ParcelHeader::ParcelHeader() : seq_num_(GenerateSeqNum()) { + clock_gettime(CLOCK_MONOTONIC_RAW, &time_stamp_); +} + +void ParcelHeader::WriteToParcel(tizen_base::Parcel* parcel) const { + parcel->WriteInt32(seq_num_); + parcel->WriteInt64(static_cast(time_stamp_.tv_sec)); + parcel->WriteInt64(static_cast(time_stamp_.tv_nsec)); +} + +void ParcelHeader::ReadFromParcel(tizen_base::Parcel* parcel) { + parcel->ReadInt32(&seq_num_); + parcel->ReadInt64(reinterpret_cast(&time_stamp_.tv_sec)); + parcel->ReadInt64(reinterpret_cast(&time_stamp_.tv_nsec)); +} + +void ParcelHeader::SetSeqNum(int seq_num) { + seq_num_ = seq_num; +} + +int ParcelHeader::GetSeqNum() const { + return seq_num_; +} + +struct timespec ParcelHeader::GetTimeStamp() const { + return time_stamp_; +} + +int ParcelHeader::GenerateSeqNum() { + static std::atomic num { 0 }; + constexpr int limit = 0x3fffffff; + ++num; + return num &= limit; +} + +} // namespace internal +} // namespace rpc_port diff --git a/src/parcel-header-internal.hh b/src/parcel-header-internal.hh new file mode 100644 index 0000000..3519e2d --- /dev/null +++ b/src/parcel-header-internal.hh @@ -0,0 +1,48 @@ +/* + * Copyright (c) 2021 Samsung Electronics Co., Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef PARCEL_HEADER_INTERNAL_H_ +#define PARCEL_HEADER_INTERNAL_H_ + +#include + +#include + +namespace rpc_port { +namespace internal { + +class ParcelHeader : public tizen_base::Parcelable { + public: + ParcelHeader(); + + void WriteToParcel(tizen_base::Parcel* parcel) const override; + void ReadFromParcel(tizen_base::Parcel* parcel) override; + + void SetSeqNum(int seq_num); + int GetSeqNum() const; + struct timespec GetTimeStamp() const; + + static int GenerateSeqNum(); + + private: + int seq_num_; + struct timespec time_stamp_; +}; + +} // namespace internal +} // namespace rpc_port + +#endif // PARCEL_HEADER_INTERNAL_H_ diff --git a/src/parcel-internal.cc b/src/parcel-internal.cc new file mode 100644 index 0000000..ba6ba66 --- /dev/null +++ b/src/parcel-internal.cc @@ -0,0 +1,71 @@ +/* + * Copyright (c) 2021 Samsung Electronics Co., Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include "log-private.hh" +#include "parcel-internal.hh" + +namespace rpc_port { +namespace internal { + +Parcel::Parcel() { + parcel_create(&handle_); +} + +Parcel::~Parcel() { + if (handle_) + parcel_destroy(handle_); +} + +void Parcel::WriteToParcel(tizen_base::Parcel* parcel) const { + parcel->WriteParcelable(header_); + + void* raw = nullptr; + uint32_t size = 0; + parcel_get_raw(handle_, &raw, &size); + parcel->WriteUInt32(size); + parcel->Write(raw, size); +} + +void Parcel::ReadFromParcel(tizen_base::Parcel* parcel) { + parcel->ReadParcelable(&header_); + + uint32_t size = 0; + parcel->ReadUInt32(&size); + if (size > 0) { + auto buf = new (std::nothrow) uint8_t[size]; + if (buf == nullptr) { + _E("Out of memory"); + return; + } + + std::unique_ptr ptr(buf); + parcel->Read(buf, size); + parcel_reset(handle_, buf, size); + } +} + +const ParcelHeader* Parcel::GetParcelHeader() { + return &header_; +} + +parcel_h Parcel::GetHandle() const { + return handle_; +} + +} // namespace internal +} // namespace rpc_port diff --git a/src/parcel-internal.hh b/src/parcel-internal.hh new file mode 100644 index 0000000..77507fb --- /dev/null +++ b/src/parcel-internal.hh @@ -0,0 +1,48 @@ +/* + * Copyright (c) 2021 Samsung Electronics Co., Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef PARCEL_INTERNAL_HH_ +#define PARCEL_INTERNAL_HH_ + +#include + +#include + +#include "parcel-header-internal.hh" + +namespace rpc_port { +namespace internal { + +class Parcel : public tizen_base::Parcelable { + public: + Parcel(); + ~Parcel(); + + void WriteToParcel(tizen_base::Parcel* parcel) const override; + void ReadFromParcel(tizen_base::Parcel* parcel) override; + + const ParcelHeader* GetParcelHeader(); + parcel_h GetHandle() const; + + private: + ParcelHeader header_; + parcel_h handle_ = nullptr; +}; + +} // namespace internal +} // namespace rpc_port + +#endif // PARCEL_INTERNAL_HH_ diff --git a/src/rpc-port-parcel.cc b/src/rpc-port-parcel.cc index 424d781..ded14b2 100644 --- a/src/rpc-port-parcel.cc +++ b/src/rpc-port-parcel.cc @@ -14,7 +14,7 @@ * limitations under the License. */ -#include +#include #include #include @@ -22,6 +22,7 @@ #include "include/rpc-port-parcel.h" #include "log-private.hh" +#include "parcel-internal.hh" #include "port-internal.hh" #define MAX_PARCEL_SIZE (1024 * 1024 * 10) @@ -31,26 +32,13 @@ using namespace rpc_port; -static int __convert_parcel_error(int error) { - switch (error) { - case PARCEL_ERROR_ILLEGAL_BYTE_SEQ: - case PARCEL_ERROR_NO_DATA: - return RPC_PORT_ERROR_IO_ERROR; - case PARCEL_ERROR_OUT_OF_MEMORY: - return RPC_PORT_ERROR_OUT_OF_MEMORY; - default: - return RPC_PORT_ERROR_INVALID_PARAMETER; - } -} - RPC_API int rpc_port_parcel_create(rpc_port_parcel_h* h) { if (h == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = nullptr; - int ret = parcel_create(&parcel); - if (ret != PARCEL_ERROR_NONE) - return __convert_parcel_error(ret); + auto* parcel = new (std::nothrow) internal::Parcel(); + if (parcel == nullptr) + return RPC_PORT_ERROR_OUT_OF_MEMORY; *h = static_cast(parcel); return RPC_PORT_ERROR_NONE; @@ -74,7 +62,12 @@ RPC_API int rpc_port_parcel_create_from_port(rpc_port_parcel_h* h, if (len <= 0 || len > MAX_PARCEL_SIZE) return RPC_PORT_ERROR_IO_ERROR; - buf = new unsigned char[len]; + buf = new (std::nothrow) unsigned char[len]; + if (buf == nullptr) { + _E("Out of memory"); + return RPC_PORT_ERROR_IO_ERROR; + } + ret = rpc_port_read(port, buf, len); if (ret != 0) { delete[] buf; @@ -82,14 +75,15 @@ RPC_API int rpc_port_parcel_create_from_port(rpc_port_parcel_h* h, } } - parcel_h parcel = nullptr; - int ret = parcel_create(&parcel); - if (ret != PARCEL_ERROR_NONE) { + auto* parcel = new (std::nothrow) internal::Parcel(); + if (parcel == nullptr) { + _E("Out of memory"); delete[] buf; return RPC_PORT_ERROR_IO_ERROR; } - parcel_burst_write(parcel, buf, len); + tizen_base::Parcel raw_parcel(buf, len); + raw_parcel.ReadParcelable(parcel); delete[] buf; *h = static_cast(parcel); return RPC_PORT_ERROR_NONE; @@ -99,11 +93,12 @@ RPC_API int rpc_port_parcel_send(rpc_port_parcel_h h, rpc_port_h port) { if (h == nullptr || port == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = static_cast(h); - void* raw = nullptr; - uint32_t len = 0; - parcel_get_raw(parcel, &raw, &len); - + auto* parcel = static_cast(h); + tizen_base::Parcel raw_parcel; + raw_parcel.WriteParcelable(*parcel); + const std::vector& raw_data = raw_parcel.GetRaw(); + void* raw = reinterpret_cast(const_cast(&raw_data[0])); + uint32_t len = raw_data.size(); if (len <= 0) return RPC_PORT_ERROR_INVALID_PARAMETER; @@ -126,8 +121,8 @@ RPC_API int rpc_port_parcel_destroy(rpc_port_parcel_h h) { if (h == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = static_cast(h); - parcel_destroy(parcel); + auto* parcel = static_cast(h); + delete parcel; return RPC_PORT_ERROR_NONE; } @@ -135,8 +130,8 @@ RPC_API int rpc_port_parcel_write_byte(rpc_port_parcel_h h, char b) { if (h == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = static_cast(h); - parcel_write_byte(parcel, b); + auto* parcel = static_cast(h); + parcel_write_byte(parcel->GetHandle(), b); return RPC_PORT_ERROR_NONE; } @@ -144,8 +139,8 @@ RPC_API int rpc_port_parcel_write_int16(rpc_port_parcel_h h, short i) { if (h == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = static_cast(h); - parcel_write_int16(parcel, i); + auto* parcel = static_cast(h); + parcel_write_int16(parcel->GetHandle(), i); return RPC_PORT_ERROR_NONE; } @@ -153,8 +148,8 @@ RPC_API int rpc_port_parcel_write_int32(rpc_port_parcel_h h, int i) { if (h == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = static_cast(h); - parcel_write_int32(parcel, i); + auto* parcel = static_cast(h); + parcel_write_int32(parcel->GetHandle(), i); return RPC_PORT_ERROR_NONE; } @@ -162,8 +157,8 @@ RPC_API int rpc_port_parcel_write_int64(rpc_port_parcel_h h, long long i) { if (h == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = static_cast(h); - parcel_write_int64(parcel, i); + auto* parcel = static_cast(h); + parcel_write_int64(parcel->GetHandle(), i); return RPC_PORT_ERROR_NONE; } @@ -171,8 +166,8 @@ RPC_API int rpc_port_parcel_write_float(rpc_port_parcel_h h, float f) { if (h == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = static_cast(h); - parcel_write_float(parcel, f); + auto* parcel = static_cast(h); + parcel_write_float(parcel->GetHandle(), f); return RPC_PORT_ERROR_NONE; } @@ -180,8 +175,8 @@ RPC_API int rpc_port_parcel_write_double(rpc_port_parcel_h h, double d) { if (h == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = static_cast(h); - parcel_write_double(parcel, d); + auto* parcel = static_cast(h); + parcel_write_double(parcel->GetHandle(), d); return RPC_PORT_ERROR_NONE; } @@ -189,8 +184,8 @@ RPC_API int rpc_port_parcel_write_string(rpc_port_parcel_h h, const char* str) { if (h == nullptr || str == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = static_cast(h); - parcel_write_string(parcel, str); + auto* parcel = static_cast(h); + parcel_write_string(parcel->GetHandle(), str); return RPC_PORT_ERROR_NONE; } @@ -198,8 +193,8 @@ RPC_API int rpc_port_parcel_write_bool(rpc_port_parcel_h h, bool b) { if (h == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = static_cast(h); - parcel_write_bool(parcel, b); + auto* parcel = static_cast(h); + parcel_write_bool(parcel->GetHandle(), b); return RPC_PORT_ERROR_NONE; } @@ -212,8 +207,8 @@ RPC_API int rpc_port_parcel_write_bundle(rpc_port_parcel_h h, bundle* b) { bundle_encode(b, &raw, &len); auto ptr = std::unique_ptr(raw, std::free); - parcel_h parcel = static_cast(h); - parcel_write_string(parcel, reinterpret_cast(raw)); + auto* parcel = static_cast(h); + parcel_write_string(parcel->GetHandle(), reinterpret_cast(raw)); return RPC_PORT_ERROR_NONE; } @@ -221,8 +216,8 @@ RPC_API int rpc_port_parcel_write_array_count(rpc_port_parcel_h h, int count) { if (h == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = static_cast(h); - parcel_write_int32(parcel, count); + auto* parcel = static_cast(h); + parcel_write_int32(parcel->GetHandle(), count); return RPC_PORT_ERROR_NONE; } @@ -242,8 +237,8 @@ RPC_API int rpc_port_parcel_read_byte(rpc_port_parcel_h h, char* b) { if (h == nullptr || b == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = static_cast(h); - int ret = parcel_read_byte(parcel, b); + auto* parcel = static_cast(h); + int ret = parcel_read_byte(parcel->GetHandle(), b); if (ret != PARCEL_ERROR_NONE) _E("parcel_read_byte() is failed. error(%d)", ret); @@ -254,8 +249,8 @@ RPC_API int rpc_port_parcel_read_int16(rpc_port_parcel_h h, short* i) { if (h == nullptr || i == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = static_cast(h); - int ret = parcel_read_int16(parcel, i); + auto* parcel = static_cast(h); + int ret = parcel_read_int16(parcel->GetHandle(), i); if (ret != PARCEL_ERROR_NONE) _E("parcel_read_int16() is failed. error(%d)", ret); @@ -266,8 +261,8 @@ RPC_API int rpc_port_parcel_read_int32(rpc_port_parcel_h h, int* i) { if (h == nullptr || i == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = static_cast(h); - int ret = parcel_read_int32(parcel, i); + auto* parcel = static_cast(h); + int ret = parcel_read_int32(parcel->GetHandle(), i); if (ret != PARCEL_ERROR_NONE) _E("parcel_read_int32() is failed. error(%d)", ret); @@ -278,9 +273,9 @@ RPC_API int rpc_port_parcel_read_int64(rpc_port_parcel_h h, long long* i) { if (h == nullptr || i == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = static_cast(h); + auto* parcel = static_cast(h); int64_t val = 0; - int ret = parcel_read_int64(parcel, &val); + int ret = parcel_read_int64(parcel->GetHandle(), &val); if (ret != PARCEL_ERROR_NONE) _E("parcel_read_int64() is failed. error(%d)", ret); @@ -292,8 +287,8 @@ RPC_API int rpc_port_parcel_read_float(rpc_port_parcel_h h, float* f) { if (h == nullptr || f == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = static_cast(h); - int ret = parcel_read_float(parcel, f); + auto* parcel = static_cast(h); + int ret = parcel_read_float(parcel->GetHandle(), f); if (ret != PARCEL_ERROR_NONE) _E("parcel_read_float() is failed. error(%d)", ret); @@ -304,8 +299,8 @@ RPC_API int rpc_port_parcel_read_double(rpc_port_parcel_h h, double* d) { if (h == nullptr || d == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = static_cast(h); - int ret = parcel_read_double(parcel, d); + auto* parcel = static_cast(h); + int ret = parcel_read_double(parcel->GetHandle(), d); if (ret != PARCEL_ERROR_NONE) _E("parcel_read_double() is failed. error(%d)", ret); @@ -316,8 +311,8 @@ RPC_API int rpc_port_parcel_read_string(rpc_port_parcel_h h, char** str) { if (h == nullptr || str == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = static_cast(h); - int ret = parcel_read_string(parcel, str); + auto* parcel = static_cast(h); + int ret = parcel_read_string(parcel->GetHandle(), str); if (ret != PARCEL_ERROR_NONE) { _E("parcel_read_string() is failed. error(%d)", ret); *str = strdup(""); @@ -330,9 +325,9 @@ RPC_API int rpc_port_parcel_read_bool(rpc_port_parcel_h h, bool* b) { if (h == nullptr || b == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = static_cast(h); - int ret = parcel_read_bool(parcel, b); - if (ret != PARCEL_ERROR_NONE) + auto* parcel = static_cast(h); + int ret = parcel_read_bool(parcel->GetHandle(), b); + if (ret != 0) _E("parcel_read_bool() is failed. error(%d)", ret); return RPC_PORT_ERROR_NONE; @@ -342,10 +337,10 @@ RPC_API int rpc_port_parcel_read_bundle(rpc_port_parcel_h h, bundle** b) { if (h == nullptr || b == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = static_cast(h); + auto* parcel = static_cast(h); char* raw = nullptr; - int ret = parcel_read_string(parcel, &raw); - if (ret != PARCEL_ERROR_NONE) { + int ret = parcel_read_string(parcel->GetHandle(), &raw); + if (ret != 0) { _E("parcel_read_string() is failed. error(%d)", ret); *b = bundle_create(); } else { @@ -360,9 +355,9 @@ RPC_API int rpc_port_parcel_read_array_count(rpc_port_parcel_h h, int* count) { if (h == nullptr || count == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = static_cast(h); - int ret = parcel_read_int32(parcel, count); - if (ret != PARCEL_ERROR_NONE) + auto* parcel = static_cast(h); + int ret = parcel_read_int32(parcel->GetHandle(), count); + if (ret != 0) _E("parcel_read_int32() is failed. error(%d)", ret); return RPC_PORT_ERROR_NONE; @@ -377,7 +372,6 @@ RPC_API int rpc_port_parcel_read(rpc_port_parcel_h h, return RPC_PORT_ERROR_INVALID_PARAMETER; parcelable->from(h, data); - return RPC_PORT_ERROR_NONE; } @@ -386,11 +380,12 @@ RPC_API int rpc_port_parcel_burst_read(rpc_port_parcel_h h, unsigned char *buf, if (h == nullptr || buf == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = static_cast(h); + auto* parcel = static_cast(h); uint32_t valid_size = size & UINT32_MAX; - int ret = parcel_burst_read(parcel, static_cast(buf), valid_size); + int ret = parcel_burst_read(parcel->GetHandle(), static_cast(buf), + valid_size); if (ret != PARCEL_ERROR_NONE) - _E("parcel_burst_read() is failed. error(%d)", ret); + _E("parcel_read() is failed. error(%d)", ret); return RPC_PORT_ERROR_NONE; } @@ -400,9 +395,10 @@ RPC_API int rpc_port_parcel_burst_write(rpc_port_parcel_h h, if (h == nullptr || buf == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = static_cast(h); + auto* parcel = static_cast(h); uint32_t valid_size = size & UINT32_MAX; - parcel_burst_write(parcel, static_cast(buf), valid_size); + parcel_burst_write(parcel->GetHandle(), static_cast(buf), + valid_size); return RPC_PORT_ERROR_NONE; } @@ -410,8 +406,8 @@ RPC_API int rpc_port_parcel_reset_reader(rpc_port_parcel_h h) { if (h == nullptr) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = static_cast(h); - parcel_reset_reader(parcel); + auto* parcel = static_cast(h); + parcel_reset_reader(parcel->GetHandle()); return RPC_PORT_ERROR_NONE; } @@ -420,10 +416,14 @@ RPC_API int rpc_port_parcel_to_array(rpc_port_parcel_h h, void** array, if (h == nullptr || !array || !size) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = static_cast(h); + auto* parcel = static_cast(h); void* raw = nullptr; uint32_t raw_size = 0; - parcel_get_raw(parcel, &raw, &raw_size); + parcel_get_raw(parcel->GetHandle(), &raw, &raw_size); + if (raw_size == 0) { + _E("raw_size is zero"); + return RPC_PORT_ERROR_IO_ERROR; + } void* array_ptr = malloc(raw_size); if (array_ptr == nullptr) @@ -437,11 +437,53 @@ RPC_API int rpc_port_parcel_to_array(rpc_port_parcel_h h, void** array, RPC_API int rpc_port_parcel_from_array(rpc_port_parcel_h h, const void* array, unsigned int size) { - if (h == nullptr || !array || size == 0) + if (h == nullptr || array == nullptr || size == 0) return RPC_PORT_ERROR_INVALID_PARAMETER; - parcel_h parcel = static_cast(h); + auto* parcel = static_cast(h); uint32_t valid_size = size & UINT32_MAX; - parcel_reset(parcel, array, valid_size); + parcel_reset(parcel->GetHandle(), array, valid_size); + return RPC_PORT_ERROR_NONE; +} + +RPC_API int rpc_port_parcel_get_header(rpc_port_parcel_h h, + rpc_port_parcel_header_h* header) { + if (h == nullptr || header == nullptr) + return RPC_PORT_ERROR_INVALID_PARAMETER; + + auto* parcel = static_cast(h); + auto* parcel_header = parcel->GetParcelHeader(); + *header = reinterpret_cast( + const_cast(parcel_header)); + return RPC_PORT_ERROR_NONE; +} + +RPC_API int rpc_port_parcel_header_set_seq_num(rpc_port_parcel_header_h header, + int seq_num) { + if (header == nullptr) + return RPC_PORT_ERROR_INVALID_PARAMETER; + + auto* parcel_header = static_cast(header); + parcel_header->SetSeqNum(seq_num); + return RPC_PORT_ERROR_NONE; +} + +RPC_API int rpc_port_parcel_header_get_seq_num(rpc_port_parcel_header_h header, + int* seq_num) { + if (header == nullptr || seq_num == nullptr) + return RPC_PORT_ERROR_INVALID_PARAMETER; + + auto* parcel_header = static_cast(header); + *seq_num = parcel_header->GetSeqNum(); + return RPC_PORT_ERROR_NONE; +} + +RPC_API int rpc_port_parcel_header_get_timestamp( + rpc_port_parcel_header_h header, struct timespec* timestamp) { + if (header == nullptr || timestamp == nullptr) + return RPC_PORT_ERROR_INVALID_PARAMETER; + + auto* parcel_header = static_cast(header); + *timestamp = parcel_header->GetTimeStamp(); return RPC_PORT_ERROR_NONE; } diff --git a/test/unit_tests/rpc_port_parcel_test.cc b/test/unit_tests/rpc_port_parcel_test.cc index 2b67502..79bb7dd 100644 --- a/test/unit_tests/rpc_port_parcel_test.cc +++ b/test/unit_tests/rpc_port_parcel_test.cc @@ -320,3 +320,85 @@ TEST_F(ParcelTest, rpc_port_parcel_burst_read_write_P) { ASSERT_EQ(buf[i], buf2[i]); } } + +/* + * @testcase rpc_port_parcel_get_header_P + * @description Gets the header handle from the rpc port parcel handle + * @apicovered rpc_port_parcel_get_header + */ +TEST_F(ParcelTest, rpc_port_parcel_get_header_P) { + rpc_port_parcel_header_h header = nullptr; + int ret = rpc_port_parcel_get_header(handle_, &header); + ASSERT_EQ(ret, RPC_PORT_ERROR_NONE); + ASSERT_NE(header, nullptr); +} + +/* + * @testcase rpc_port_parcel_get_header_N + * @description Gets the header handle from the rpc port parcel handle + * @apicovered rpc_port_parcel_get_header + */ +TEST_F(ParcelTest, rpc_port_parcel_get_header_N) { + int ret = rpc_port_parcel_get_header(nullptr, nullptr); + ASSERT_EQ(ret, RPC_PORT_ERROR_INVALID_PARAMETER); +} + +/* + * @testcase rpc_port_parcel_header_set_get_seq_num_P + * @description Sets and Gets the sequence number + * @apicovered rpc_port_parcel_header_set_seq_num, rpc_port_parcel_header_get_seq_num + */ +TEST_F(ParcelTest, rpc_port_parcel_header_set_get_seq_num_P) { + rpc_port_parcel_header_h header = nullptr; + int ret = rpc_port_parcel_get_header(handle_, &header); + ASSERT_EQ(ret, RPC_PORT_ERROR_NONE); + + ret = rpc_port_parcel_header_set_seq_num(header, 100); + ASSERT_EQ(ret, RPC_PORT_ERROR_NONE); + + int seq_num = 0; + ret = rpc_port_parcel_header_get_seq_num(header, &seq_num); + ASSERT_EQ(ret, RPC_PORT_ERROR_NONE); + ASSERT_EQ(seq_num, 100); +} + +/* + * @testcase rpc_port_parcel_header_set_get_seq_num_N + * @description Sets and Gets the sequence number + * @apicovered rpc_port_parcel_header_set_seq_num, rpc_port_parcel_header_get_seq_num + */ +TEST_F(ParcelTest, rpc_port_parcel_header_set_get_seq_num_N) { + int ret = rpc_port_parcel_header_set_seq_num(nullptr, -1); + ASSERT_EQ(ret, RPC_PORT_ERROR_INVALID_PARAMETER); + + ret = rpc_port_parcel_header_get_seq_num(nullptr, nullptr); + ASSERT_EQ(ret, RPC_PORT_ERROR_INVALID_PARAMETER); +} + +/* + * @testcase rpc_port_parcel_header_get_timestamp_P + * @description Gets the timestamp from the header handle of the rpc port parcel + * @apicovered rpc_port_parcel_header_get_timestamp + */ +TEST_F(ParcelTest, rpc_port_parcel_header_get_timestamp_P) { + rpc_port_parcel_header_h header = nullptr; + int ret = rpc_port_parcel_get_header(handle_, &header); + ASSERT_EQ(ret, RPC_PORT_ERROR_NONE); + + struct timespec timestamp = { 0, }; + ret = rpc_port_parcel_header_get_timestamp(header, ×tamp); + ASSERT_EQ(ret, RPC_PORT_ERROR_NONE); + + ASSERT_NE(timestamp.tv_sec, 0); + ASSERT_NE(timestamp.tv_nsec, 0); +} + +/* + * @testcase rpc_port_parcel_header_get_timestamp_N + * @description Gets the timestamp from the header handle of the rpc port parcel + * @apicovered rpc_port_parcel_header_get_timestamp + */ +TEST_F(ParcelTest, rpc_port_parcel_header_get_timestamp_N) { + int ret = rpc_port_parcel_header_get_timestamp(nullptr, nullptr); + ASSERT_EQ(ret, RPC_PORT_ERROR_INVALID_PARAMETER); +} -- 2.7.4 From 006b4211ee45a008078b7e58cd7f2b66e239b468 Mon Sep 17 00:00:00 2001 From: Changgyu Choi Date: Thu, 5 Aug 2021 13:37:59 +0900 Subject: [PATCH 09/16] Add disconnect API Adds: - rpc_port_disconnect() Change-Id: I189f0ef05a0ae63c9f8258f17e741eeba8fc96a2 Signed-off-by: Changgyu Choi --- include/rpc-port.h | 11 +++++++++ src/rpc-port.cc | 10 ++++++++ test/unit_tests/rpc_port_test.cc | 52 +++++++++++++++++++++++++++++++++------- 3 files changed, 65 insertions(+), 8 deletions(-) diff --git a/include/rpc-port.h b/include/rpc-port.h index 86b175d..bde887d 100644 --- a/include/rpc-port.h +++ b/include/rpc-port.h @@ -536,6 +536,17 @@ int rpc_port_set_private_sharing(rpc_port_h port, const char *path); int rpc_port_unset_private_sharing(rpc_port_h port); /** + * @brief Disconnects the port. + * @since_tizen 6.5 + * @param[in] port The rpc port handle + * @return @c 0 on success, + * otherwise a negative error value + * @retval #RPC_PORT_ERROR_NONE Successful + * @retval #RPC_PORT_ERROR_INVALID_PARAMETER Invalid parameter + */ +int rpc_port_disconnect(rpc_port_h h); + +/** * @} */ diff --git a/src/rpc-port.cc b/src/rpc-port.cc index 310cfbc..2ee572d 100644 --- a/src/rpc-port.cc +++ b/src/rpc-port.cc @@ -544,3 +544,13 @@ RPC_API int rpc_port_unset_private_sharing(rpc_port_h h) { return port->UnsetPrivateSharing(); } + +RPC_API int rpc_port_disconnect(rpc_port_h h) { + if (h == nullptr) + return RPC_PORT_ERROR_INVALID_PARAMETER; + + auto port = static_cast(h); + port->Disconnect(); + + return RPC_PORT_ERROR_NONE; +} diff --git a/test/unit_tests/rpc_port_test.cc b/test/unit_tests/rpc_port_test.cc index 7163202..e738121 100644 --- a/test/unit_tests/rpc_port_test.cc +++ b/test/unit_tests/rpc_port_test.cc @@ -352,10 +352,15 @@ class RpcPortConnection : public RpcPortBase { ret = rpc_port_stub_add_disconnected_event_cb(stub_handle_, [](const char* sender, const char* instance, void *data) { - RpcPortConnection* p = static_cast(data); + auto* p = static_cast(data); p->touch_stub_disconnected_event_cb_ = true; - p->Finish(); - }, this); + g_timeout_add(1, [](void* data) -> gboolean { + auto* p = static_cast(data); + p->Finish(); + return G_SOURCE_REMOVE; + }, p); + }, + this); ASSERT_EQ(ret, 0); ret = rpc_port_stub_add_privilege(stub_handle_, @@ -368,7 +373,7 @@ class RpcPortConnection : public RpcPortBase { void ProxySetup() { int ret = rpc_port_proxy_add_connected_event_cb(proxy_handle_, - [](const char *ep, const char *port_name, rpc_port_h port, void *data) { + [](const char* ep, const char* port_name, rpc_port_h port, void* data) { RpcPortConnection* p = static_cast(data); p->proxy_port_ = port; rpc_port_proxy_get_port(p->proxy_handle_, RPC_PORT_PORT_CALLBACK, @@ -378,11 +383,16 @@ class RpcPortConnection : public RpcPortBase { ASSERT_EQ(ret, 0); ret = rpc_port_proxy_add_disconnected_event_cb(proxy_handle_, - [](const char *ep, const char *port_name, void *data) { - RpcPortConnection* p = static_cast(data); + [](const char* ep, const char* port_name, void* data) { + auto* p = static_cast(data); p->touch_proxy_disconnected_event_cb_ = true; - p->Finish(); - }, this); + g_timeout_add(1, [](void* data) -> gboolean { + auto* p = static_cast(data); + p->Finish(); + return G_SOURCE_REMOVE; + }, p); + }, + this); ASSERT_EQ(ret, 0); ret = rpc_port_proxy_add_received_event_cb(proxy_handle_, @@ -616,3 +626,29 @@ TEST_F(RpcPortConnection, rpc_port_stub_disconnected_N) { ASSERT_TRUE(touch_stub_disconnected_event_cb_); } + +/* + * @testcase rpc_port_disconnect_P + * @description disconnect the port. + * And then, checks whether the disconnected event callback is invoked or not. + * @apicovered rpc_port_disconnect + */ +TEST_F(RpcPortConnection, rpc_port_disconnect_P) { + char res[] = "test"; + if (proxy_port_ == nullptr) + RunMainLoop(); + + ASSERT_NE(proxy_port_, nullptr); + int ret = rpc_port_write(proxy_port_, res, sizeof(res)); + ASSERT_EQ(ret, 0); + + if (stub_port_ == nullptr) + RunMainLoop(); + + ret = rpc_port_disconnect(stub_port_); + EXPECT_EQ(ret, 0); + + RunMainLoop(); + ASSERT_TRUE(touch_stub_disconnected_event_cb_); + ASSERT_TRUE(touch_proxy_disconnected_event_cb_); +} -- 2.7.4 From 08ea8bb3eca6b0a9e56c51f09e5f77bf89af3176 Mon Sep 17 00:00:00 2001 From: Changgyu Choi Date: Fri, 13 Aug 2021 15:06:13 +0900 Subject: [PATCH 10/16] Release version 1.10.0 Changes: - Add the header handle of the parcel - Add disconnect API Change-Id: Iee2dddd83ee7a25018a1d6130f6a58c8a7962512 Signed-off-by: Changgyu Choi --- 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 b2ee437..6930a90 100644 --- a/packaging/rpc-port.spec +++ b/packaging/rpc-port.spec @@ -1,6 +1,6 @@ Name: rpc-port Summary: RPC Port library -Version: 1.9.4 +Version: 1.10.0 Release: 0 Group: Application Framework/Libraries License: Apache-2.0 -- 2.7.4 From f61ad32b21308885a2eb6411b4cf69f7e6c464ea Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Tue, 17 Aug 2021 11:04:25 +0900 Subject: [PATCH 11/16] Use INT_MAX instead of custom variable Change-Id: I9ec72a5c9591ffbfc91b9f6203fc1bf70eeb4087 Signed-off-by: Hwankyu Jhun --- src/parcel-header-internal.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/parcel-header-internal.cc b/src/parcel-header-internal.cc index c1d51fb..12cf541 100644 --- a/src/parcel-header-internal.cc +++ b/src/parcel-header-internal.cc @@ -15,6 +15,7 @@ */ #include +#include #include "parcel-header-internal.hh" @@ -51,9 +52,8 @@ struct timespec ParcelHeader::GetTimeStamp() const { int ParcelHeader::GenerateSeqNum() { static std::atomic num { 0 }; - constexpr int limit = 0x3fffffff; ++num; - return num &= limit; + return num &= INT_MAX; } } // namespace internal -- 2.7.4 From 7e782b560198a5baab88dd9fec89ef4d2a1526e5 Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Tue, 17 Aug 2021 11:06:23 +0900 Subject: [PATCH 12/16] Fix a bug about ReadParcelable Impl In 32-bit machine, the long size is 4 bytes. When calling the ReadInt64() with timespec.tv_nsec. the buffer overflow is occurred. Change-Id: Id5bcb26891c63b893b20d50e7dab4c38dbaea14b Signed-off-by: Hwankyu Jhun --- src/parcel-header-internal.cc | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/parcel-header-internal.cc b/src/parcel-header-internal.cc index 12cf541..888b13a 100644 --- a/src/parcel-header-internal.cc +++ b/src/parcel-header-internal.cc @@ -16,6 +16,7 @@ #include #include +#include #include "parcel-header-internal.hh" @@ -34,8 +35,14 @@ void ParcelHeader::WriteToParcel(tizen_base::Parcel* parcel) const { void ParcelHeader::ReadFromParcel(tizen_base::Parcel* parcel) { parcel->ReadInt32(&seq_num_); - parcel->ReadInt64(reinterpret_cast(&time_stamp_.tv_sec)); - parcel->ReadInt64(reinterpret_cast(&time_stamp_.tv_nsec)); + + int64_t tv_sec = 0; + parcel->ReadInt64(&tv_sec); + time_stamp_.tv_sec = tv_sec & std::numeric_limits::max(); + + int64_t tv_nsec = 0; + parcel->ReadInt64(&tv_nsec); + time_stamp_.tv_nsec = tv_nsec & std::numeric_limits::max(); } void ParcelHeader::SetSeqNum(int seq_num) { -- 2.7.4 From 02567231334ded62ea06506e0204001b6571039f Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Tue, 17 Aug 2021 16:47:31 +0900 Subject: [PATCH 13/16] Release version 1.10.1 Changes: - Use INT_MAX instead of custom variable - Fix a bug about ReadParcelable Impl Change-Id: I0100382e69183b2b8d95648358ac81c934123dee 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 6930a90..37ed830 100644 --- a/packaging/rpc-port.spec +++ b/packaging/rpc-port.spec @@ -1,6 +1,6 @@ Name: rpc-port Summary: RPC Port library -Version: 1.10.0 +Version: 1.10.1 Release: 0 Group: Application Framework/Libraries License: Apache-2.0 -- 2.7.4 From 1520ff9f66b23b4e6c6fb7a399c1e0bc53f3d673 Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Tue, 17 Aug 2021 13:07:53 +0900 Subject: [PATCH 14/16] Make thread safe codes The rpc_port_stub_h handle destroys in the main thread. The rpc-port releases the handle using g_idle_add_full(). Before, the idler is added, the Ignore() method is called to set nullptr to the event listener. While calling the callback function, the rpc-port checks whether the event listener is nullptr or not. If it's nullptr, the callback function will be returned immediately. Change-Id: I7728552c38173c548c07ce52c1339c6da9c4b29b Signed-off-by: Hwankyu Jhun --- src/rpc-port.cc | 31 +++++++++++++++++++++++++++++-- src/stub-internal.cc | 43 ++++++++++++++++++++++++------------------- src/stub-internal.hh | 13 +++++++------ 3 files changed, 60 insertions(+), 27 deletions(-) diff --git a/src/rpc-port.cc b/src/rpc-port.cc index 2ee572d..d02fc44 100644 --- a/src/rpc-port.cc +++ b/src/rpc-port.cc @@ -140,7 +140,7 @@ class ProxyExt : public Proxy, public Proxy::IEventListener { class StubExt : public Stub, public Stub::IEventListener { public: - explicit StubExt(const std::string& port) : Stub(port) {} + explicit StubExt(const std::string& port) : Stub(port), destroying_(false) {} virtual ~StubExt() = default; void AddConnectedEventListener(rpc_port_stub_connected_event_cb cb, @@ -163,6 +163,9 @@ class StubExt : public Stub, public Stub::IEventListener { void OnConnected(const std::string& sender, const std::string& instance) override { + if (IsDestroying()) + return; + for (auto& ev : connected_events_) { ev->cb_(sender.c_str(), instance.c_str(), ev->user_data_); } @@ -170,6 +173,9 @@ class StubExt : public Stub, public Stub::IEventListener { void OnDisconnected(const std::string& sender, const std::string& instance) override { + if (IsDestroying()) + return; + for (auto& ev : disconnected_events_) { ev->cb_(sender.c_str(), instance.c_str(), ev->user_data_); } @@ -177,6 +183,9 @@ class StubExt : public Stub, public Stub::IEventListener { int OnReceived(const std::string& sender, const std::string& instance, Port* port) override { + if (IsDestroying()) + return -1; + for (auto& ev : received_events_) { int ret = ev->cb_(sender.c_str(), instance.c_str(), port, ev->user_data_); @@ -187,11 +196,20 @@ class StubExt : public Stub, public Stub::IEventListener { return 0; } + void SetDestroying(bool destroying) { + destroying_ = destroying; + } + + bool IsDestroying() { + return destroying_; + } + std::recursive_mutex& GetMutex() const { return mutex_; } private: + std::atomic destroying_; std::list>> connected_events_; std::list>> @@ -406,7 +424,16 @@ RPC_API int rpc_port_stub_destroy(rpc_port_stub_h h) { _W("rpc_port_stub_destroy(%p)", h); auto p = static_cast<::StubExt*>(h); aul_rpc_port_usr_destroy(p->GetPortName().c_str(), rpc_port_get_target_uid()); - delete p; + p->Ignore(); + p->SetDestroying(true); + + g_idle_add_full(G_PRIORITY_HIGH, + [](gpointer data) -> gboolean { + auto p = static_cast<::StubExt*>(data); + _W("rpc_port_stub_destroy(%p)", p); + delete p; + return G_SOURCE_REMOVE; + }, h, nullptr); return RPC_PORT_ERROR_NONE; } diff --git a/src/stub-internal.cc b/src/stub-internal.cc index 553a9a6..48fe4a3 100644 --- a/src/stub-internal.cc +++ b/src/stub-internal.cc @@ -114,6 +114,11 @@ int Stub::Listen(IEventListener* ev, int fd) { return server_->Listen(); } +void Stub::Ignore() { + std::lock_guard lock(GetMutex()); + listener_ = nullptr; +} + void Stub::AddPrivilege(const std::string& privilege) { std::lock_guard lock(GetMutex()); access_controller_.AddPrivilege(privilege); @@ -165,9 +170,9 @@ void Stub::RemoveAcceptedPorts(std::string instance) { } } -gboolean Stub::OnDataReceived(GIOChannel* channel, GIOCondition cond, - gpointer user_data) { - Stub* stub = static_cast(user_data); +gboolean Stub::AcceptedPort::OnDataReceived(GIOChannel* channel, + GIOCondition cond, gpointer user_data) { + auto* stub = static_cast(user_data); std::lock_guard lock(stub->GetMutex()); auto* listener = stub->listener_; if (listener == nullptr) { @@ -184,7 +189,7 @@ gboolean Stub::OnDataReceived(GIOChannel* channel, GIOCondition cond, listener->OnDisconnected(p->GetId(), p->GetInstance()); stub->RemoveAcceptedPorts(p->GetInstance()); Aul::NotifyRpcFinished(); - return G_SOURCE_REMOVE; + return G_SOURCE_CONTINUE; } int ret = stub->listener_->OnReceived(p->GetId(), p->GetInstance(), @@ -194,7 +199,7 @@ gboolean Stub::OnDataReceived(GIOChannel* channel, GIOCondition cond, listener->OnDisconnected(p->GetId(), p->GetInstance()); stub->RemoveAcceptedPorts(p->GetInstance()); Aul::NotifyRpcFinished(); - return G_SOURCE_REMOVE; + return G_SOURCE_CONTINUE; } break; @@ -204,8 +209,8 @@ gboolean Stub::OnDataReceived(GIOChannel* channel, GIOCondition cond, return G_SOURCE_CONTINUE; } -gboolean Stub::OnSocketDisconnected(GIOChannel* channel, GIOCondition cond, - gpointer user_data) { +gboolean Stub::AcceptedPort::OnSocketDisconnected(GIOChannel* channel, + GIOCondition cond, gpointer user_data) { auto* stub = static_cast(user_data); std::lock_guard lock(stub->GetMutex()); auto* listener = stub->listener_; @@ -277,11 +282,11 @@ Stub::AcceptedPort::AcceptedPort(Stub* parent, bool isDelegate, int fd, } Stub::AcceptedPort::~AcceptedPort() { - if (disconn_src_ > 0) - g_source_remove(disconn_src_); + if (disconn_source_ > 0) + g_source_remove(disconn_source_); - if (src_ > 0) - g_source_remove(src_); + if (source_ > 0) + g_source_remove(source_); if (channel_ != nullptr) g_io_channel_unref(channel_); @@ -294,10 +299,10 @@ int Stub::AcceptedPort::Watch(bool receive) { return -1; } - disconn_src_ = g_io_add_watch(channel_, + disconn_source_ = g_io_add_watch(channel_, static_cast(G_IO_ERR | G_IO_HUP | G_IO_NVAL), - Stub::OnSocketDisconnected, parent_); - if (disconn_src_ == 0) { + OnSocketDisconnected, parent_); + if (disconn_source_ == 0) { _E("g_io_add_watch() is failed"); return -1; } @@ -305,9 +310,9 @@ int Stub::AcceptedPort::Watch(bool receive) { if (!receive) return 0; - src_ = g_io_add_watch(channel_, static_cast(G_IO_IN), - Stub::OnDataReceived, parent_); - if (src_ == 0) { + source_ = g_io_add_watch(channel_, static_cast(G_IO_IN), + OnDataReceived, parent_); + if (source_ == 0) { _E("g_io_add_watch() is failed"); return -1; } @@ -347,9 +352,9 @@ int Stub::Server::Listen() { gboolean Stub::Server::OnRequestReceived(GIOChannel* channel, GIOCondition cond, gpointer user_data) { - Stub* stub = static_cast(user_data); + auto* stub = static_cast(user_data); std::lock_guard lock(stub->GetMutex()); - if (stub->server_.get() == nullptr) { + if (stub->listener_ == nullptr) { _E("Invalid context"); return G_SOURCE_REMOVE; } diff --git a/src/stub-internal.hh b/src/stub-internal.hh index 28114cc..7e32d4d 100644 --- a/src/stub-internal.hh +++ b/src/stub-internal.hh @@ -50,6 +50,7 @@ class Stub { virtual ~Stub(); int Listen(IEventListener* ev, int fd); + void Ignore(); void AddPrivilege(const std::string& privilege); void SetTrusted(const bool trusted); std::shared_ptr FindPort(const std::string& instance) const; @@ -70,11 +71,15 @@ class Stub { private: int Watch(bool receive); + static gboolean OnDataReceived(GIOChannel* channel, GIOCondition cond, + gpointer user_data); + static gboolean OnSocketDisconnected(GIOChannel* channel, GIOCondition cond, + gpointer user_data); private: GIOChannel* channel_ = nullptr; - int disconn_src_ = 0; - int src_ = 0; + guint disconn_source_ = 0; + guint source_ = 0; Stub* parent_; bool is_delegate_ = false; }; @@ -96,10 +101,6 @@ class Stub { guint source_ = 0;; }; - static gboolean OnDataReceived(GIOChannel* channel, GIOCondition cond, - gpointer user_data); - static gboolean OnSocketDisconnected(GIOChannel* channel, GIOCondition cond, - gpointer user_data); void AddAcceptedPort(const std::string& sender_appid, const std::string& instance, const std::string& port_type, int fd); void RemoveAcceptedPorts(std::string instance); -- 2.7.4 From 8e252c77f9847410a5e2e9819a530b372300341b Mon Sep 17 00:00:00 2001 From: Changgyu Choi Date: Wed, 18 Aug 2021 11:46:51 +0900 Subject: [PATCH 15/16] Fix some types Change-Id: I2a516191a72c098b8e9f89751a124c38052d66eb Signed-off-by: Changgyu Choi --- include/rpc-port.h | 2 +- test/unit_tests/rpc_port_parcel_test.cc | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/rpc-port.h b/include/rpc-port.h index bde887d..458f922 100644 --- a/include/rpc-port.h +++ b/include/rpc-port.h @@ -544,7 +544,7 @@ int rpc_port_unset_private_sharing(rpc_port_h port); * @retval #RPC_PORT_ERROR_NONE Successful * @retval #RPC_PORT_ERROR_INVALID_PARAMETER Invalid parameter */ -int rpc_port_disconnect(rpc_port_h h); +int rpc_port_disconnect(rpc_port_h port); /** * @} diff --git a/test/unit_tests/rpc_port_parcel_test.cc b/test/unit_tests/rpc_port_parcel_test.cc index 79bb7dd..c982844 100644 --- a/test/unit_tests/rpc_port_parcel_test.cc +++ b/test/unit_tests/rpc_port_parcel_test.cc @@ -345,7 +345,7 @@ TEST_F(ParcelTest, rpc_port_parcel_get_header_N) { /* * @testcase rpc_port_parcel_header_set_get_seq_num_P - * @description Sets and Gets the sequence number + * @description Sets and gets the sequence number * @apicovered rpc_port_parcel_header_set_seq_num, rpc_port_parcel_header_get_seq_num */ TEST_F(ParcelTest, rpc_port_parcel_header_set_get_seq_num_P) { @@ -364,7 +364,7 @@ TEST_F(ParcelTest, rpc_port_parcel_header_set_get_seq_num_P) { /* * @testcase rpc_port_parcel_header_set_get_seq_num_N - * @description Sets and Gets the sequence number + * @description Sets and gets the sequence number * @apicovered rpc_port_parcel_header_set_seq_num, rpc_port_parcel_header_get_seq_num */ TEST_F(ParcelTest, rpc_port_parcel_header_set_get_seq_num_N) { -- 2.7.4 From 3ea83876a332519c533a8fd632eb3ba3fa6c3936 Mon Sep 17 00:00:00 2001 From: Changgyu Choi Date: Wed, 18 Aug 2021 14:16:18 +0900 Subject: [PATCH 16/16] Fix some rpc-port-parcel API's description Change-Id: I113827fbdbee8361bf6d82556c33524c28be861b Signed-off-by: Changgyu Choi --- include/rpc-port-parcel.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/include/rpc-port-parcel.h b/include/rpc-port-parcel.h index 292dfd6..005148c 100644 --- a/include/rpc-port-parcel.h +++ b/include/rpc-port-parcel.h @@ -402,7 +402,6 @@ int rpc_port_parcel_read_array_count(rpc_port_parcel_h h, int *count); */ int rpc_port_parcel_read(rpc_port_parcel_h h, rpc_port_parcelable_t *parcelable, void *data); - /** * @brief Reads bytes from rpc port parcel handle. * @since_tizen @if MOBILE 4.0 @elseif WEARABLE 5.0 @endif @@ -447,7 +446,7 @@ int rpc_port_parcel_get_header(rpc_port_parcel_h h, rpc_port_parcel_header_h *he /** * @brief Sets the sequence number to the header handle of the rpc port parcel. * @since_tizen 6.5 - * @param[in] header The header handler of the rpc port parcel + * @param[in] header The header handle of the rpc port parcel * @param[in] seq_num The sequence number * @return @c 0 on success, * otherwise a negative error value -- 2.7.4