Skip to content

Navigation Menu

Sign in
Appearance settings
Sign in
Sign up
Appearance settings

Commit 390b5f9

Browse filesBrowse the repository at this point in the historyBrowse files
ryanofskyryanofsky
committed
Merge bitcoin-core/libmultiprocess#344: test: listen_tests and connect_tests follow-ups
c39c785 doc: note construct() call in valid init interface test (xyzconstant) b9c36c6 test: close sockets unconditionally and check errors with KJ_SYSCALL (xyzconstant) 7eb741e test: drop unnecessary KJ_EXPECT(true) (xyzconstant) 113f1d4 test: join server thread unconditionally in connect tests (xyzconstant) 44bc463 test: drop mp:: prefixes in connect tests (xyzconstant) 038d33e test: share DefaultLogHandler between test files (xyzconstant) b54a163 test: drop TestSetup socket members in connect tests (xyzconstant) 70467c5 test: add m_ prefix to TestSetup members in connect tests (xyzconstant) cc260f2 test: replace capnp fix link with upstream PR (xyzconstant) Pull request description: Addresses review suggestions left (all of them made by ryanofsky) in the now-merged PRs bitcoin#298 and bitcoin#310. These are non-critical test cleanups (naming, simplification, comments, etc.) with zero changes to library code. ACKs for top commit: ryanofsky: Code review ACK c39c785. Thanks for the followup! Tree-SHA512: e703d4508f46978c0be7831d3be04c831829aa5de08873e93fbe4ba02b0a5db71554fd80c30058336a4b3f3a620f496d5fb4bfee66f99fc24234073fa98db4c4
2 parents e18ca52 + c39c785 commit 390b5f9
Copy full SHA for 390b5f9

3 files changed

+79-95Lines changed: 79 additions & 95 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎test/mp/test/common.h‎

Copy file name to clipboard
+27Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Copyright (c) The Bitcoin Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+
#ifndef MP_TEST_COMMON_H
6+
#define MP_TEST_COMMON_H
7+
8+
#include <kj/debug.h>
9+
#include <mp/proxy-io.h>
10+
11+
#include <stdexcept>
12+
13+
namespace mp {
14+
namespace test {
15+
16+
//! Default event loop log handler used by tests. Logs all messages and throws
17+
//! on errors so calling code can assert on them.
18+
inline void DefaultLogHandler(LogMessage log)
19+
{
20+
KJ_LOG(INFO, log.level, log.message);
21+
if (log.level == Log::Raise) throw std::runtime_error(log.message);
22+
}
23+
24+
} // namespace test
25+
} // namespace mp
26+
27+
#endif // MP_TEST_COMMON_H
Collapse file

‎test/mp/test/connect_tests.cpp‎

Copy file name to clipboardExpand all lines: test/mp/test/connect_tests.cpp
+49-82Lines changed: 49 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,25 @@
11
// Copyright (c) The Bitcoin Core developers
22
// Distributed under the MIT software license, see the accompanying
33
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
#include "common.h"
45
#include "unixlistener.h"
56
#include <kj/async.h>
67
#include <kj/common.h>
78
#include <kj/debug.h>
89
#include <kj/memory.h>
910
#include <kj/test.h>
10-
#include <mp/proxy.h>
1111
#include <mp/proxy-io.h>
12+
#include <mp/proxy.h>
1213
#include <mp/test/foo.capnp.h>
1314
#include <mp/test/foo.capnp.proxy.h>
15+
#include <mp/util.h>
1416
#include <sys/socket.h>
15-
#include <sys/types.h>
1617
#include <unistd.h>
1718

19+
#include <array>
1820
#include <chrono>
1921
#include <condition_variable>
2022
#include <cstring> // IWYU pragma: keep
21-
#include <functional>
2223
#include <future>
2324
#include <memory>
2425
#include <mutex>
@@ -34,86 +35,63 @@ namespace {
3435

3536
constexpr auto FAILURE_TIMEOUT = std::chrono::seconds{30};
3637

37-
//! Default event loop log handler used by tests, throws so the calling code
38-
//! can assert on errors.
39-
void DefaultLogHandler(mp::LogMessage log)
40-
{
41-
if (log.level == mp::Log::Raise)
42-
throw std::runtime_error(log.message);
43-
}
44-
4538
class TestSetup
4639
{
4740
public:
48-
int client_fd;
49-
int server_fd;
50-
51-
mp::EventLoop* loop;
52-
std::optional<mp::EventLoopRef> loop_ref;
41+
EventLoop* m_loop;
42+
std::optional<EventLoopRef> m_loop_ref;
5343
//! Thread variable should be after other struct members so the thread does
5444
//! not start until the other members are initialized.
55-
std::thread loop_thread;
56-
57-
TestSetup(mp::LogFn log_handler = DefaultLogHandler)
58-
: TestSetup(
59-
[](int fds[2]) {
60-
KJ_REQUIRE(socketpair(AF_UNIX, SOCK_STREAM, 0, fds) != -1);
61-
},
62-
log_handler) {}
45+
std::thread m_loop_thread;
6346

64-
TestSetup(const std::function<void(int[2])>& init_sockets,
65-
mp::LogFn log_handler = DefaultLogHandler)
47+
TestSetup(LogFn log_handler = DefaultLogHandler)
6648
{
67-
std::promise<mp::EventLoop*> loop_promise;
68-
loop_thread = std::thread([&, log_handler] {
69-
mp::EventLoop loop("mptest-connect", log_handler);
49+
std::promise<EventLoop*> loop_promise;
50+
m_loop_thread = std::thread([&, log_handler] {
51+
EventLoop loop("mptest-connect", log_handler);
7052
loop_promise.set_value(&loop);
7153
loop.loop();
7254
});
73-
loop = loop_promise.get_future().get();
74-
loop_ref.emplace(*loop);
75-
76-
// Initialize and store sockets
77-
int fds[2] = {-1, -1};
78-
init_sockets(fds);
79-
80-
client_fd = fds[0];
81-
server_fd = fds[1];
55+
m_loop = loop_promise.get_future().get();
56+
m_loop_ref.emplace(*m_loop);
8257
}
8358

8459
~TestSetup()
8560
{
86-
loop_ref.reset();
87-
loop_thread.join();
61+
m_loop_ref.reset();
62+
m_loop_thread.join();
8863
}
8964
};
9065

9166
KJ_TEST("ConnectStream connects to a socket serving a valid init interface")
9267
{
9368
TestSetup setup;
69+
auto [client_fd, server_fd] = SocketPair();
9470

95-
std::thread server_thread([&setup]() {
96-
mp::EventLoop server_loop("mptest-valid-server", DefaultLogHandler);
71+
std::thread server_thread([&]() {
72+
EventLoop server_loop("mptest-valid-server", DefaultLogHandler);
9773
std::unique_ptr<FooInit> init = std::make_unique<FooInit>();
98-
ServeStream<messages::FooInit>(server_loop, MakeStream(server_loop, setup.server_fd), *init);
74+
ServeStream<messages::FooInit>(server_loop, MakeStream(server_loop, server_fd), *init);
9975
server_loop.loop();
10076
});
10177

102-
auto init = ConnectStream<messages::FooInit>(*setup.loop, MakeStream(*setup.loop, setup.client_fd));
78+
// FooInit has a `construct()` method, so this connects to the server and
79+
// sends an IPC request that must complete successfully.
80+
auto init = ConnectStream<messages::FooInit>(*setup.m_loop, MakeStream(*setup.m_loop, client_fd));
10381

10482
init.reset();
10583
server_thread.join();
106-
KJ_EXPECT(true);
10784
}
10885

10986
KJ_TEST("ConnectStream throws when the socket is already disconnected")
11087
{
11188
TestSetup setup;
89+
auto [client_fd, server_fd] = SocketPair();
11290

113-
close(setup.server_fd);
91+
KJ_SYSCALL(close(server_fd));
11492

11593
try {
116-
auto init = ConnectStream<messages::FooInit>(*setup.loop, MakeStream(*setup.loop, setup.client_fd));
94+
auto init = ConnectStream<messages::FooInit>(*setup.m_loop, MakeStream(*setup.m_loop, client_fd));
11795

11896
KJ_EXPECT(false);
11997
} catch (const std::runtime_error& e) {
@@ -126,12 +104,13 @@ KJ_TEST("ConnectStream throws when the socket is already disconnected")
126104
KJ_TEST("ConnectStream defers disconnect failure to the first IPC request for interfaces without construct()")
127105
{
128106
TestSetup setup;
107+
auto [client_fd, server_fd] = SocketPair();
129108

130-
close(setup.server_fd);
109+
KJ_SYSCALL(close(server_fd));
131110

132111
// Without a construct() method no IPC call is made during client
133112
// creation, so ConnectStream succeeds even though the peer is gone.
134-
auto foo = ConnectStream<messages::FooInterface>(*setup.loop, MakeStream(*setup.loop, setup.client_fd));
113+
auto foo = ConnectStream<messages::FooInterface>(*setup.m_loop, MakeStream(*setup.m_loop, client_fd));
135114

136115
try {
137116
foo->add(1, 2);
@@ -155,18 +134,19 @@ KJ_TEST("ConnectStream handles a disconnect when no client calls are made")
155134
std::condition_variable cv;
156135
bool warned = false;
157136

158-
TestSetup setup([&](mp::LogMessage log) {
159-
if (log.level == mp::Log::Warning && log.message.find("unexpected network disconnect") != std::string::npos) {
137+
TestSetup setup([&](LogMessage log) {
138+
if (log.level == Log::Warning && log.message.find("unexpected network disconnect") != std::string::npos) {
160139
const std::lock_guard<std::mutex> lock(mutex);
161140
warned = true;
162141
cv.notify_all();
163142
}
164143
DefaultLogHandler(log);
165144
});
145+
auto [client_fd, server_fd] = SocketPair();
166146

167-
close(setup.server_fd);
147+
KJ_SYSCALL(close(server_fd));
168148

169-
auto foo = ConnectStream<messages::FooInterface>(*setup.loop, MakeStream(*setup.loop, setup.client_fd));
149+
auto foo = ConnectStream<messages::FooInterface>(*setup.m_loop, MakeStream(*setup.m_loop, client_fd));
170150

171151
// The disconnect handler registered by ProxyClientBase should run and
172152
// delete the connection even when no calls are ever made.
@@ -177,67 +157,54 @@ KJ_TEST("ConnectStream handles a disconnect when no client calls are made")
177157
KJ_TEST("ConnectStream throws when the socket disconnects after receiving data")
178158
{
179159
TestSetup setup;
160+
auto [client_fd, server_fd] = SocketPair();
180161

181-
std::thread server_thread([&setup]() {
162+
std::thread server_thread([&]() {
182163
char buf[128];
183164

184-
ssize_t bytes_received =
185-
recv(setup.server_fd, buf, sizeof(buf), 0);
186-
187-
if (bytes_received > 0) {
188-
close(setup.server_fd);
189-
}
165+
recv(server_fd, buf, sizeof(buf), 0);
166+
KJ_SYSCALL(close(server_fd));
190167
});
191168

192169
try {
193-
auto init = ConnectStream<messages::FooInit>(*setup.loop, MakeStream(*setup.loop, setup.client_fd));
170+
auto init = ConnectStream<messages::FooInit>(*setup.m_loop, MakeStream(*setup.m_loop, client_fd));
194171

195-
if (server_thread.joinable()) server_thread.join();
196172
KJ_EXPECT(false);
197173
} catch (const std::runtime_error& e) {
198-
if (server_thread.joinable()) server_thread.join();
199-
200174
std::string_view reason = e.what();
201175
KJ_EXPECT(reason == "IPC client method call interrupted by disconnect.");
202176
}
177+
server_thread.join();
203178
}
204179

205180
KJ_TEST("ConnectStream throws when a connection accepted from a listener disconnects after receiving data")
206181
{
207182
UnixListener listener;
183+
TestSetup setup;
184+
int client_fd = listener.MakeConnectedSocket();
185+
int server_fd = listener.release();
208186

209-
TestSetup setup([&listener](int fds[2]) {
210-
fds[0] = listener.MakeConnectedSocket(); // client_fd
211-
fds[1] = listener.release(); // server_fd
212-
});
213-
214-
std::thread server_thread([&setup]() {
187+
std::thread server_thread([&]() {
215188
char buf[128];
216189

217-
int connection_fd = accept(setup.server_fd, nullptr, nullptr);
190+
int connection_fd = accept(server_fd, nullptr, nullptr);
218191

219192
if (connection_fd >= 0) {
220-
ssize_t bytes_received =
221-
recv(connection_fd, buf, sizeof(buf), 0);
222-
223-
if (bytes_received > 0) {
224-
close(connection_fd);
225-
}
193+
recv(connection_fd, buf, sizeof(buf), 0);
194+
KJ_SYSCALL(close(connection_fd));
226195
}
227-
close(setup.server_fd);
196+
KJ_SYSCALL(close(server_fd));
228197
});
229198

230199
try {
231-
auto init = ConnectStream<messages::FooInit>(*setup.loop, MakeStream(*setup.loop, setup.client_fd));
200+
auto init = ConnectStream<messages::FooInit>(*setup.m_loop, MakeStream(*setup.m_loop, client_fd));
232201

233-
if (server_thread.joinable()) server_thread.join();
234202
KJ_EXPECT(false);
235203
} catch (const std::runtime_error& e) {
236-
if (server_thread.joinable()) server_thread.join();
237-
238204
std::string_view reason = e.what();
239205
KJ_EXPECT(reason == "IPC client method call interrupted by disconnect.");
240206
}
207+
server_thread.join();
241208
}
242209

243210
} // namespace
Collapse file

‎test/mp/test/listen_tests.cpp‎

Copy file name to clipboardExpand all lines: test/mp/test/listen_tests.cpp
+3-13Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Distributed under the MIT software license, see the accompanying
33
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
44

5+
#include "common.h"
56
#include "unixlistener.h"
67
#include <mp/test/foo.capnp.h>
78
#include <mp/test/foo.capnp.proxy.h>
@@ -23,7 +24,6 @@
2324
#include <mp/util.h>
2425
#include <ratio> // IWYU pragma: keep
2526
#include <optional>
26-
#include <stdexcept>
2727
#include <string>
2828
#include <thread>
2929
#include <unistd.h>
@@ -42,10 +42,7 @@ class ClientSetup
4242
public:
4343
explicit ClientSetup(int fd)
4444
: thread([this, fd] {
45-
EventLoop loop("mptest-client", [](mp::LogMessage log) {
46-
KJ_LOG(INFO, log.level, log.message);
47-
if (log.level == mp::Log::Raise) throw std::runtime_error(log.message);
48-
});
45+
EventLoop loop("mptest-client", DefaultLogHandler);
4946
client_promise.set_value(ConnectStream<messages::FooInterface>(loop, MakeStream(loop, fd)));
5047
loop.loop();
5148
})
@@ -67,13 +64,6 @@ class ClientSetup
6764
std::thread thread;
6865
};
6966

70-
//! Default server event loop log handler, throws so tests can assert on errors.
71-
void DefaultLogHandler(mp::LogMessage log)
72-
{
73-
KJ_LOG(INFO, log.level, log.message);
74-
if (log.level == mp::Log::Raise) throw std::runtime_error(log.message);
75-
}
76-
7767
//! Runs a server EventLoop on its own thread, starts ListenConnections() on a
7868
//! UnixListener socket, and records connection/disconnection counts through
7969
//! EventLoop test hooks
@@ -241,7 +231,7 @@ KJ_TEST("ListenConnections handles a client that disconnects before being accept
241231
// The event loop then reports this as an uncaught task exception. We catch and ignore
242232
// this specific error here so that the corresponding CI job does not fail.
243233
//
244-
// This is a Cap'n Proto bug, a fix is available in the v2 branch at: https://github.com/capnproto/capnproto/commit/7df5bd078f389ded313479981bd0ae06cbcdfe1b#diff-ec577ad66535f58f6d7396ea51d3e56c0065308aa8fb02751cd6a8cfaa67252fR1358-R1372
234+
// This is a Cap'n Proto bug, fixed by https://github.com/capnproto/capnproto/pull/2748
245235
if (log.level == mp::Log::Error && log.message.find("Uncaught exception in daemonized task.") != std::string::npos) {
246236
Lock lock(mutex);
247237
accept_error = true;

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.