From a3ad231fac16efbfe4603e063cc2e50ece245f42 Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Fri, 6 Aug 2021 10:43:14 +0900 Subject: [PATCH] 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