From 836abb300ce09b5c5abb7bcd13f04e8c6bd20de3 Mon Sep 17 00:00:00 2001 From: bneradt Date: Thu, 30 Jul 2026 14:31:51 -0500 Subject: [PATCH] Fix JSONRPC server shutdown race JSONRPC server shutdown can race with worker thread startup. When the worker starts after stop_thread(), it restores the running flag and polls a closed socket indefinitely. This causes test_jsonrpcserver and process shutdown to hang. This marks the socket server as running before the worker is created, so a concurrent stop cannot be overwritten. It also passes the owning server to the worker instead of relying on the mutable global server pointer. --- include/mgmt/rpc/server/IPCSocketServer.h | 2 +- src/mgmt/rpc/server/IPCSocketServer.cc | 6 ++++-- src/mgmt/rpc/server/RPCServer.cc | 12 ++++++------ 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/include/mgmt/rpc/server/IPCSocketServer.h b/include/mgmt/rpc/server/IPCSocketServer.h index 4ffe377d43c..7c45204b13f 100644 --- a/include/mgmt/rpc/server/IPCSocketServer.h +++ b/include/mgmt/rpc/server/IPCSocketServer.h @@ -144,7 +144,7 @@ class IPCSocketServer : public BaseCommInterface void close(); void late_check_peer_credentials(int peedFd, TSRPCHandlerOptions const &options, swoc::Errata &errata) const; - std::atomic_bool _running; + std::atomic_bool _running{false}; struct sockaddr_un _serverAddr; int _socket{-1}; diff --git a/src/mgmt/rpc/server/IPCSocketServer.cc b/src/mgmt/rpc/server/IPCSocketServer.cc index e2b235650b0..922db9b8ec3 100644 --- a/src/mgmt/rpc/server/IPCSocketServer.cc +++ b/src/mgmt/rpc/server/IPCSocketServer.cc @@ -168,6 +168,10 @@ IPCSocketServer::init() return ec; } + // Set this before RPCServer creates the worker thread so an immediate stop + // cannot be overwritten when the worker eventually enters run(). + _running.store(true); + return ec; } @@ -201,8 +205,6 @@ IPCSocketServer::poll_for_new_client(std::chrono::milliseconds timeout) const void IPCSocketServer::run() { - _running.store(true); - while (_running) { // poll till socket it's ready. if (!this->poll_for_new_client()) { diff --git a/src/mgmt/rpc/server/RPCServer.cc b/src/mgmt/rpc/server/RPCServer.cc index eb121359169..6c8a55afa6a 100644 --- a/src/mgmt/rpc/server/RPCServer.cc +++ b/src/mgmt/rpc/server/RPCServer.cc @@ -59,13 +59,13 @@ RPCServer::~RPCServer() void * /* static */ RPCServer::run_thread(void *a) { - void *ret = a; - if (jsonrpcServer->_init) { - jsonrpcServer->_rpcThread = jsonrpcServer->_init(); + auto *server = static_cast(a); + if (server->_init) { + server->_rpcThread = server->_init(); } - jsonrpcServer->_socketImpl->run(); + server->_socketImpl->run(); Dbg(dbg_ctl, "Socket stopped"); - return ret; + return a; } void @@ -75,7 +75,7 @@ RPCServer::start_thread(std::function const &cb_init, std::function< _init = cb_init; _destroy = cb_destroy; - ink_thread_create(&_this_thread, run_thread, nullptr, 0, 0, nullptr); + ink_thread_create(&_this_thread, run_thread, this, 0, 0, nullptr); } void