btcsignals: use a single shared_ptr for liveness and callback
What changed, and why it matters
This is a small internal code cleanup in Bitcoin Core's custom signal/slot system. It merges two separate shared pointers (one tracking whether a callback is still active, and one used as a construction tag) into a single shared pointer. There is no user-facing change, no bug fix, and no security-relevant behavior change visible in the diff.
No security action required. Treat as normal code-quality refactoring.
Security signals we found
No security-relevant behavioral change
Refactoring / code simplification only
No new inputs or trust boundaries
No memory safety bug fixed or introduced in diff
Evidence from the diff
The commit refactors src/btcsignals.h. Previously, connection held a std::shared_ptr
Changed components
src/btcsignals.hBtcSignals signal/slot utilityInspect captured patch +24 / −23
diff --git a/src/btcsignals.h b/src/btcsignals.h
index f4c2530b..ebe7a5ac 100644
--- a/src/btcsignals.h
+++ b/src/btcsignals.h
@@ -56,23 +56,28 @@ class connection
{
template <typename Signature, typename Combiner>
friend class signal;
-
- /*
- * Tag for the constructor used by signal.
+ /**
+ * Track liveness. Also serves as a tag for the constructor used by signal.
*/
- struct enabled_tag_type {
+ class liveness
+ {
+ friend class connection;
+ std::atomic_bool m_connected{true};
+
+ void disconnect() { m_connected.store(false); }
+ public:
+ bool connected() const { return m_connected.load(); }
};
- static constexpr enabled_tag_type enabled_tag{};
/**
* connections have shared_ptr-like copy and move semantics.
*/
- std::shared_ptr<std::atomic_bool> m_connected{};
+ std::shared_ptr<liveness> m_state{};
/**
* Only a signal can create an enabled connection.
*/
- explicit connection(enabled_tag_type /*unused*/) : m_connected{std::make_shared<std::atomic_bool>(true)} {}
+ explicit connection(std::shared_ptr<liveness>&& state) : m_state{std::move(state)}{}
public:
/**
@@ -92,8 +97,8 @@ public:
*/
void disconnect()
{
- if (m_connected) {
- m_connected->store(false);
+ if (m_state) {
+ m_state->disconnect();
}
}
@@ -103,7 +108,7 @@ public:
*/
bool connected() const
{
- return m_connected && m_connected->load();
+ return m_state && m_state->connected();
}
};
@@ -148,23 +153,19 @@ class signal
static_assert(std::is_same_v<Combiner, optional_last_value<typename function_type::result_type>>, "only the optional_last_value combiner is supported");
/*
- * Helper struct for maintaining a callback and its associated connection
+ * Helper struct for maintaining a callback and its associated connection liveness
*/
- struct connection_holder {
+ struct connection_holder : connection::liveness {
template <typename Callable>
connection_holder(Callable&& callback) : m_callback{std::forward<Callable>(callback)}
{
}
- connection m_connection{connection::enabled_tag};
- function_type m_callback;
+ const function_type m_callback;
};
mutable Mutex m_mutex;
- /* Store connection_holders as shared_ptrs to avoid having to copy them by
- * value in operator().
- */
std::vector<std::shared_ptr<connection_holder>> m_connections GUARDED_BY(m_mutex){};
public:
@@ -208,14 +209,14 @@ public:
}
if constexpr (std::is_void_v<result_type>) {
for (const auto& connection : connections) {
- if (connection->m_connection.connected()) {
+ if (connection->connected()) {
connection->m_callback(args...);
}
}
} else {
result_type ret{std::nullopt};
for (const auto& connection : connections) {
- if (connection->m_connection.connected()) {
+ if (connection->connected()) {
ret.emplace(connection->m_callback(args...));
}
}
@@ -233,10 +234,10 @@ public:
LOCK(m_mutex);
// Garbage-collect disconnected connections to prevent unbounded growth
- std::erase_if(m_connections, [](const auto& holder) { return !holder->m_connection.connected(); });
+ std::erase_if(m_connections, [](const auto& holder) { return !holder->connected(); });
- const auto& connection = m_connections.emplace_back(std::make_shared<connection_holder>(std::forward<Callable>(func)));
- return connection->m_connection;
+ const auto& entry = m_connections.emplace_back(std::make_shared<connection_holder>(std::forward<Callable>(func)));
+ return connection(entry);
}
/*
@@ -246,7 +247,7 @@ public:
{
LOCK(m_mutex);
return std::ranges::none_of(m_connections, [](const auto& holder) {
- return holder->m_connection.connected();
+ return holder->connected();
});
}
};
Why this scored 13/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.