Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor sockets #614

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ BITCOIN_CORE_H = \
undo.h \
util.h \
utilmoneystr.h \
util/sock.h \
utilstrencodings.h \
utiltime.h \
validationinterface.h \
Expand Down Expand Up @@ -397,6 +398,7 @@ libbitcoin_common_a_SOURCES = \
script/script_error.cpp \
script/sign.cpp \
script/standard.cpp \
util/sock.cpp \
$(BITCOIN_CORE_H) \
$(LIBZCASH_H)

Expand Down
1 change: 1 addition & 0 deletions src/Makefile.gtest.include
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ zen_gtest_SOURCES += \
gtest/test_pow.cpp \
gtest/test_random.cpp \
gtest/test_rpc.cpp \
gtest/test_sock.cpp \
gtest/test_getblocktemplate.cpp \
gtest/test_timedata.cpp \
gtest/test_transaction.cpp \
Expand Down
15 changes: 15 additions & 0 deletions src/compat.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,14 @@ typedef u_int SOCKET;
#define THREAD_PRIORITY_ABOVE_NORMAL (-2)
#endif

// The type of the option value passed to getsockopt & setsockopt
// differs between Windows and non-Windows.
#ifndef WIN32
typedef void* sockopt_arg_type;
#else
typedef char* sockopt_arg_type;
#endif

#if HAVE_DECL_STRNLEN == 0
size_t strnlen( const char *start, size_t max_len);
#endif // HAVE_DECL_STRNLEN
Expand All @@ -101,4 +109,11 @@ bool static inline IsSelectableSocket(SOCKET s) {
#endif
}

// Note these both should work with the current usage of poll, but best to be safe
// WIN32 poll is broken https://daniel.haxx.se/blog/2012/10/10/wsapoll-is-broken/
// __APPLE__ poll is broke https://github.com/bitcoin/bitcoin/pull/14336#issuecomment-437384408
#if defined(__linux__)
#define USE_POLL
#endif

#endif // BITCOIN_COMPAT_H
2 changes: 1 addition & 1 deletion src/gtest/test_asyncproofverifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class AsyncProofVerifierTestSuite : public ::testing::Test
mempool.reset(new CTxMemPool(::minRelayTxFee, DEFAULT_MAX_MEMPOOL_SIZE_MB * 1000000));
connman.reset(new CConnman());

dummyNode.reset(new CNode(INVALID_SOCKET, CAddress(), "", true));
dummyNode.reset(new CNode(nullptr, CAddress(), "", true));
dummyNode->id = 7;

sidechain.creationBlockHeight = 100;
Expand Down
2 changes: 1 addition & 1 deletion src/gtest/test_mempool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ class CNodeExt : public CNode
}

CNodeExt():
CNode(INVALID_SOCKET, CAddress(ip(0xa0b0c002)), "", true)
CNode(nullptr, CAddress(ip(0xa0b0c002)), "", true)
{
}

Expand Down
120 changes: 120 additions & 0 deletions src/gtest/test_sock.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
#include <gtest/gtest.h>
#include <thread>

#include "compat.h"
#include "util/sock.h"

static bool SocketIsClosed(const SOCKET& s)
{
// Notice that if another thread is running and creates its own socket after `s` has been
// closed, it may be assigned the same file descriptor number. In this case, our test will
// wrongly pretend that the socket is not closed.
int type;
socklen_t len = sizeof(type);
return getsockopt(s, SOL_SOCKET, SO_TYPE, (sockopt_arg_type)&type, &len) == SOCKET_ERROR;
}

static SOCKET CreateSocket()
{
const SOCKET s = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
assert(s != static_cast<SOCKET>(SOCKET_ERROR));
return s;
}


TEST(Sock, ConstructorDestructor) {
SOCKET s = CreateSocket();
{
Sock sock(s);
ASSERT_EQ(sock.Get(), s);
ASSERT_FALSE(SocketIsClosed(s));
}
ASSERT_TRUE(SocketIsClosed(s));
}

TEST(Sock, MoveConstructor) {
SOCKET s = CreateSocket();
Sock sock(s);
ASSERT_EQ(sock.Get(), s);
ASSERT_FALSE(SocketIsClosed(s));

Sock sock2(std::move(sock));
ASSERT_EQ(sock.Get(), INVALID_SOCKET);
ASSERT_EQ(sock2.Get(), s);
ASSERT_FALSE(SocketIsClosed(s));
}

TEST(Sock, MoveAssignment) {
SOCKET s = CreateSocket();
Sock sock(s);
ASSERT_EQ(sock.Get(), s);
ASSERT_FALSE(SocketIsClosed(s));

Sock sock2 = std::move(sock);
ASSERT_EQ(sock.Get(), INVALID_SOCKET);
ASSERT_EQ(sock2.Get(), s);
ASSERT_FALSE(SocketIsClosed(s));
}

TEST(Sock, Reset) {
const SOCKET s = CreateSocket();
Sock sock(s);
ASSERT_FALSE(SocketIsClosed(s));
sock.Reset();
ASSERT_TRUE(SocketIsClosed(s));
}

#ifndef WIN32 // Windows does not have socketpair(2).

static void CreateSocketPair(int s[2]) {
assert(socketpair(AF_UNIX, SOCK_STREAM, 0, s) == 0);
}

static void SendAndRecvMessage(const Sock& sender, const Sock& receiver) {
const char* msg = "abcd";
constexpr size_t msg_len = 4;
char recv_buf[10];

ASSERT_EQ(sender.Send(msg, msg_len, 0), msg_len);
ASSERT_EQ(receiver.Recv(recv_buf, sizeof(recv_buf), 0), msg_len);
ASSERT_EQ(strncmp(msg, recv_buf, msg_len), 0);
}

TEST(Sock, SendAndReceive) {
int s[2];
CreateSocketPair(s);

{
Sock sock0(s[0]);
Sock sock1(s[1]);

SendAndRecvMessage(sock0, sock1);

Sock sock0moved = std::move(sock0);
Sock sock1moved = std::move(sock1);

SendAndRecvMessage(sock1moved, sock0moved);
}

ASSERT_TRUE(SocketIsClosed(s[0]));
ASSERT_TRUE(SocketIsClosed(s[1]));
}

TEST(Sock, Wait)
{
int s[2];
CreateSocketPair(s);

Sock sock0(s[0]);
Sock sock1(s[1]);

constexpr int64_t millis_in_day = 24 * 60 * 60 * 1000;
std::thread waiter([&sock0]() { ASSERT_EQ(sock0.Wait(millis_in_day, Sock::RECV), 1); });

ASSERT_EQ(sock1.Send("a", 1, 0), 1);

waiter.join();
}

#endif /* WIN32 */

2 changes: 1 addition & 1 deletion src/metrics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ int printStats(bool mining)
{
LOCK2(cs_main, connman->cs_vNodes);
connections = connman->vNodes.size();
tlsConnections = std::count_if(connman->vNodes.begin(), connman->vNodes.end(), [](CNode* n) {return n->ssl != NULL;});
tlsConnections = std::count_if(connman->vNodes.begin(), connman->vNodes.end(), [](CNode* n) {return n->GetSSL() != nullptr;});
}
unsigned long mempool_count = mempool->size();
/*
Expand Down
Loading