torcontrol: Move tor controller into node context
What changed, and why it matters
This commit is a straightforward internal code cleanup: it moves the Tor controller object from a single global variable into the per-node context structure. There is no change in user-facing behavior, no bug fix, and no security-relevant change visible in the diff.
No security action required; treat as normal refactoring review.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch refactors TorController lifecycle management. Previously a static global g_tor_controller in torcontrol.cpp was created/interrupted/stopped via free functions StartTorControl, InterruptTorControl, and StopTorControl. The commit removes those free functions and the global, instead storing a std::unique_ptr<TorController> inside NodeContext. init.cpp now directly constructs, interrupts, joins, and resets node.tor_controller. The logic and timing are preserved; only ownership and visibility changed.
Changed components
src/init.cppsrc/node/context.cppsrc/node/context.hsrc/torcontrol.cppsrc/torcontrol.hInspect captured patch +11 / −36
diff --git a/src/init.cpp b/src/init.cpp
index fb4b8fa9..f52d8d8d 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -276,7 +276,9 @@ void Interrupt(NodeContext& node)
InterruptHTTPRPC();
InterruptRPC();
InterruptREST();
- InterruptTorControl();
+ if (node.tor_controller) {
+ node.tor_controller->Interrupt();
+ }
InterruptMapPort();
if (node.connman)
node.connman->Interrupt();
@@ -319,7 +321,10 @@ void Shutdown(NodeContext& node)
if (node.peerman && node.validation_signals) node.validation_signals->UnregisterValidationInterface(node.peerman.get());
if (node.connman) node.connman->Stop();
- StopTorControl();
+ if (node.tor_controller) {
+ node.tor_controller->Join();
+ node.tor_controller.reset();
+ }
if (node.background_init_thread.joinable()) node.background_init_thread.join();
// After everything has been shut down, but before things get flushed, stop the
@@ -2187,7 +2192,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
"for the automatically created Tor onion service."),
onion_service_target.ToStringAddrPort()));
}
- StartTorControl(onion_service_target);
+ node.tor_controller = std::make_unique<TorController>(gArgs.GetArg("-torcontrol", DEFAULT_TOR_CONTROL), onion_service_target);
}
if (connOptions.bind_on_any) {
diff --git a/src/node/context.cpp b/src/node/context.cpp
index d7d01b49..16436160 100644
--- a/src/node/context.cpp
+++ b/src/node/context.cpp
@@ -17,6 +17,7 @@
#include <node/warnings.h>
#include <policy/fees/block_policy_estimator.h>
#include <scheduler.h>
+#include <torcontrol.h>
#include <txmempool.h>
#include <validation.h>
#include <validationinterface.h>
diff --git a/src/node/context.h b/src/node/context.h
index 3a7488fd..848c872f 100644
--- a/src/node/context.h
+++ b/src/node/context.h
@@ -25,6 +25,7 @@ class ChainstateManager;
class ECC_Context;
class NetGroupManager;
class PeerManager;
+class TorController;
namespace interfaces {
class Chain;
class ChainClient;
@@ -69,6 +70,7 @@ struct NodeContext {
std::unique_ptr<const NetGroupManager> netgroupman;
std::unique_ptr<CBlockPolicyEstimator> fee_estimator;
std::unique_ptr<PeerManager> peerman;
+ std::unique_ptr<TorController> tor_controller;
std::unique_ptr<ChainstateManager> chainman;
std::unique_ptr<BanMan> banman;
ArgsManager* args{nullptr}; // Currently a raw pointer because the memory is not managed by this struct
diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp
index e8e680d7..ecc75f15 100644
--- a/src/torcontrol.cpp
+++ b/src/torcontrol.cpp
@@ -735,35 +735,6 @@ fs::path TorController::GetPrivateKeyFile()
return gArgs.GetDataDirNet() / "onion_v3_private_key";
}
-/****** Thread ********/
-
-/**
- * TODO: TBD if introducing a global is the preferred approach here since we
- * usually try to avoid them. We could let init manage the lifecycle or make
- * this a part of NodeContext maybe instead.
- */
-static std::unique_ptr<TorController> g_tor_controller;
-
-void StartTorControl(CService onion_service_target)
-{
- assert(!g_tor_controller);
- g_tor_controller = std::make_unique<TorController>(gArgs.GetArg("-torcontrol", DEFAULT_TOR_CONTROL), onion_service_target);
-}
-
-void InterruptTorControl()
-{
- if (!g_tor_controller) return;
- LogInfo("tor: Thread interrupt");
- g_tor_controller->Interrupt();
-}
-
-void StopTorControl()
-{
- if (!g_tor_controller) return;
- g_tor_controller->Join();
- g_tor_controller.reset();
-}
-
CService DefaultOnionServiceTarget(uint16_t port)
{
struct in_addr onion_service_target;
diff --git a/src/torcontrol.h b/src/torcontrol.h
index 7dfc6207..75b81b87 100644
--- a/src/torcontrol.h
+++ b/src/torcontrol.h
@@ -31,10 +31,6 @@ constexpr int TOR_REPLY_OK{250};
constexpr int TOR_REPLY_UNRECOGNIZED{510};
constexpr int TOR_REPLY_SYNTAX_ERROR{512}; //!< Syntax error in command argument
-void StartTorControl(CService onion_service_target);
-void InterruptTorControl();
-void StopTorControl();
-
CService DefaultOnionServiceTarget(uint16_t port);
/** Reply from Tor, can be single or multi-line */
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.