Make DynSock accepted sockets queue optional, with precise lifetime
What the developer wrote
Authored by Matthew Zipkin
Make DynSock accepted sockets queue optional, with precise lifetime
When DynSock is used to represent a connected socket (e.g. a client)
the data I/O pipes are needed but not the m_accepted_sockets Queue,
because connected sockets do not create more connected sockets.
When DynSock is used to represent a listening socket, the Queue
is necessary to create connected sockets upon mocked connection, but
the Queue does not need to be a std::shared_ptr as long as it
is guaranteed to live as long as the DynSock.
Co-Authored by: Hodlinator <172445034+hodlinator@users.noreply.github.com>
What changed, and why it matters
This change refactors a test-only mock socket class (DynSock) used in Bitcoin Core's unit tests. It makes the queue of accepted sockets optional for connected-socket mocks and switches from a shared pointer to a raw pointer with a lifetime guarantee. It also adds an assertion to catch accidental calls to Accept() on non-listening mock sockets. The change only affects test code, not the live Bitcoin network or wallet software.
No security action required. Treat as normal code-quality/test-maintenance review. If reviewing, verify the LIFETIMEBOUND annotation is respected by all callers and that no tests rely on the previous shared_ptr semantics for queue lifetime extension.
Security signals we found
Test-only code change (src/test/util/net.cpp, src/test/util/net.h)
Added assertion to catch programming error (Accept on non-listening DynSock)
Lifetime annotation LIFETIMEBOUND added for raw pointer parameter
No changes to consensus, networking, wallet, or cryptographic code
Evidence from the diff
DynSock is a mock Sock implementation in src/test/util/net.{h,cpp} used by tests. The patch splits the constructor: one for listening sockets taking a Queue (annotated LIFETIMEBOUND), and one for connected sockets with no queue. m_accept_sockets changes from std::shared_ptr
Changed components
src/test/util/net.hsrc/test/util/net.cppInspect captured patch +19 / −4
diff --git a/src/test/util/net.cpp b/src/test/util/net.cpp
index 73e5a58d..15903ac9 100644
--- a/src/test/util/net.cpp
+++ b/src/test/util/net.cpp
@@ -346,11 +346,16 @@ void DynSock::Pipe::WaitForDataOrEof(UniqueLock<Mutex>& lock)
});
}
-DynSock::DynSock(std::shared_ptr<Pipes> pipes, std::shared_ptr<Queue> accept_sockets)
+DynSock::DynSock(std::shared_ptr<Pipes> pipes, Queue* accept_sockets)
: m_pipes{pipes}, m_accept_sockets{accept_sockets}
{
}
+DynSock::DynSock(std::shared_ptr<Pipes> pipes)
+ : m_pipes{pipes}, m_accept_sockets{}
+{
+}
+
DynSock::~DynSock()
{
m_pipes->send.Eof();
@@ -369,6 +374,7 @@ ssize_t DynSock::Send(const void* buf, size_t len, int) const
std::unique_ptr<Sock> DynSock::Accept(sockaddr* addr, socklen_t* addr_len) const
{
+ assert(m_accept_sockets && "Accept() called on non-listening DynSock");
ZeroSock::Accept(addr, addr_len);
return m_accept_sockets->Pop().value_or(nullptr);
}
@@ -403,7 +409,7 @@ bool DynSock::WaitMany(std::chrono::milliseconds timeout, EventsPerSock& events_
if ((events.requested & Sock::RECV) != 0) {
auto dyn_sock = reinterpret_cast<const DynSock*>(sock.get());
uint8_t b;
- if (dyn_sock->m_pipes->recv.GetBytes(&b, 1, MSG_PEEK) == 1 || !dyn_sock->m_accept_sockets->Empty()) {
+ if (dyn_sock->m_pipes->recv.GetBytes(&b, 1, MSG_PEEK) == 1 || (dyn_sock->m_accept_sockets && !dyn_sock->m_accept_sockets->Empty())) {
events.occurred |= Sock::RECV;
at_least_one_event_occurred = true;
}
diff --git a/src/test/util/net.h b/src/test/util/net.h
index 247eba8e..8954e631 100644
--- a/src/test/util/net.h
+++ b/src/test/util/net.h
@@ -5,6 +5,7 @@
#ifndef BITCOIN_TEST_UTIL_NET_H
#define BITCOIN_TEST_UTIL_NET_H
+#include <attributes.h>
#include <compat/compat.h>
#include <netmessagemaker.h>
#include <net.h>
@@ -336,7 +337,15 @@ public:
* @param[in] pipes Send/recv pipes used by the Send() and Recv() methods.
* @param[in] accept_sockets Sockets to return by the Accept() method.
*/
- explicit DynSock(std::shared_ptr<Pipes> pipes, std::shared_ptr<Queue> accept_sockets);
+ explicit DynSock(std::shared_ptr<Pipes> pipes, Queue* accept_sockets LIFETIMEBOUND);
+
+ /**
+ * Create a new mocked sock that represents a connected socket. It has pipes
+ * for data transport but there is no queue because connected sockets do
+ * not introduce new connected sockets.
+ * @param[in] pipes Send/recv pipes used by the Send() and Recv() methods.
+ */
+ explicit DynSock(std::shared_ptr<Pipes> pipes);
~DynSock();
@@ -356,7 +365,7 @@ private:
DynSock& operator=(Sock&&) override;
std::shared_ptr<Pipes> m_pipes;
- std::shared_ptr<Queue> m_accept_sockets;
+ Queue* const m_accept_sockets;
};
template <typename... Args>
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.