Skip to content

Commit cd03513

Browse files
committed
init: Signal-safe instant shutdown
Replace the 200ms polling loop with a faster and more efficient waiting operation. This was tried a few times before, but given up every time because solutions use a condition variable which is not safe for use in signals as they need to be reentrant. On UNIX-ish OSes, use a safe way: a pipe. When shutdown is requested write a dummy byte to the pipe. Waiting for shutdown is a matter of a blocking read from the pipe. On Windows, there are no signals so using a condition variable is safe.
1 parent 16b31cc commit cd03513

File tree

4 files changed

+112
-14
lines changed

4 files changed

+112
-14
lines changed

src/bitcoind.cpp

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,6 @@
2828
const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
2929
UrlDecodeFn* const URL_DECODE = urlDecode;
3030

31-
static void WaitForShutdown(NodeContext& node)
32-
{
33-
while (!ShutdownRequested())
34-
{
35-
UninterruptibleSleep(std::chrono::milliseconds{200});
36-
}
37-
Interrupt(node);
38-
}
39-
4031
static bool AppInit(int argc, char* argv[])
4132
{
4233
NodeContext node;
@@ -147,12 +138,10 @@ static bool AppInit(int argc, char* argv[])
147138
PrintExceptionContinue(nullptr, "AppInit()");
148139
}
149140

150-
if (!fRet)
151-
{
152-
Interrupt(node);
153-
} else {
154-
WaitForShutdown(node);
141+
if (fRet) {
142+
WaitForShutdown();
155143
}
144+
Interrupt(node);
156145
Shutdown(node);
157146

158147
return fRet;

src/init.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -917,6 +917,9 @@ bool AppInitBasicSetup(const ArgsManager& args)
917917
// Enable heap terminate-on-corruption
918918
HeapSetInformation(nullptr, HeapEnableTerminationOnCorruption, nullptr, 0);
919919
#endif
920+
if (!InitShutdownState()) {
921+
return InitError(Untranslated("Initializing wait-for-shutdown state failed."));
922+
}
920923

921924
if (!SetupNetworking()) {
922925
return InitError(Untranslated("Initializing networking failed."));

src/shutdown.cpp

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,108 @@
55

66
#include <shutdown.h>
77

8+
#include <config/bitcoin-config.h>
9+
10+
#include <assert.h>
811
#include <atomic>
12+
#ifdef WIN32
13+
#include <condition_variable>
14+
#else
15+
#include <errno.h>
16+
#include <fcntl.h>
17+
#include <unistd.h>
18+
#endif
919

1020
static std::atomic<bool> fRequestShutdown(false);
21+
#ifdef WIN32
22+
/** On windows it is possible to simply use a condition variable. */
23+
std::mutex g_shutdown_mutex;
24+
std::condition_variable g_shutdown_cv;
25+
#else
26+
/** On UNIX-like operating systems use the self-pipe trick.
27+
* Index 0 will be the read end of the pipe, index 1 the write end.
28+
*/
29+
static int g_shutdown_pipe[2] = {-1, -1};
30+
#endif
31+
32+
bool InitShutdownState()
33+
{
34+
#ifndef WIN32
35+
#if HAVE_O_CLOEXEC
36+
// If we can, make sure that the file descriptors are closed on exec()
37+
// to prevent interference.
38+
if (pipe2(g_shutdown_pipe, O_CLOEXEC) != 0) {
39+
return false;
40+
}
41+
#else
42+
if (pipe(g_shutdown_pipe) != 0) {
43+
return false;
44+
}
45+
#endif
46+
#endif
47+
return true;
48+
}
1149

1250
void StartShutdown()
1351
{
52+
#ifdef WIN32
53+
std::unique_lock<std::mutex> lk(g_shutdown_mutex);
1454
fRequestShutdown = true;
55+
g_shutdown_cv.notify_one();
56+
#else
57+
// This must be reentrant and safe for calling in a signal handler, so using a condition variable is not safe.
58+
// Make sure that the token is only written once even if multiple threads call this concurrently or in
59+
// case of a reentrant signal.
60+
if (!fRequestShutdown.exchange(true)) {
61+
// Write an arbitrary byte to the write end of the shutdown pipe.
62+
const char token = 'x';
63+
while (true) {
64+
int result = write(g_shutdown_pipe[1], &token, 1);
65+
if (result < 0) {
66+
// Failure. It's possible that the write was interrupted by another signal.
67+
// Other errors are unexpected here.
68+
assert(errno == EINTR);
69+
} else {
70+
assert(result == 1);
71+
break;
72+
}
73+
}
74+
}
75+
#endif
1576
}
77+
1678
void AbortShutdown()
1779
{
80+
if (fRequestShutdown) {
81+
// Cancel existing shutdown by waiting for it, this will reset condition flags and remove
82+
// the shutdown token from the pipe.
83+
WaitForShutdown();
84+
}
1885
fRequestShutdown = false;
1986
}
87+
2088
bool ShutdownRequested()
2189
{
2290
return fRequestShutdown;
2391
}
92+
93+
void WaitForShutdown()
94+
{
95+
#ifdef WIN32
96+
std::unique_lock<std::mutex> lk(g_shutdown_mutex);
97+
g_shutdown_cv.wait(lk, [] { return fRequestShutdown.load(); });
98+
#else
99+
char token;
100+
while (true) {
101+
int result = read(g_shutdown_pipe[0], &token, 1);
102+
if (result < 0) {
103+
// Failure. Check if the read was interrupted by a signal.
104+
// Other errors are unexpected here.
105+
assert(errno == EINTR);
106+
} else {
107+
assert(result == 1);
108+
break;
109+
}
110+
}
111+
#endif
112+
}

src/shutdown.h

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,25 @@
66
#ifndef BITCOIN_SHUTDOWN_H
77
#define BITCOIN_SHUTDOWN_H
88

9+
/** Initialize shutdown state. This must be called before using either StartShutdown(),
10+
* AbortShutdown() or WaitForShutdown(). Calling ShutdownRequested() is always safe.
11+
*/
12+
bool InitShutdownState();
13+
14+
/** Request shutdown of the application. */
915
void StartShutdown();
16+
17+
/** Clear shutdown flag. Only use this during init (before calling WaitForShutdown in any
18+
* thread), or in the unit tests. Calling it in other circumstances will cause a race condition.
19+
*/
1020
void AbortShutdown();
21+
22+
/** Returns true if a shutdown is requested, false otherwise. */
1123
bool ShutdownRequested();
1224

25+
/** Wait for StartShutdown to be called in any thread. This can only be used
26+
* from a single thread.
27+
*/
28+
void WaitForShutdown();
29+
1330
#endif

0 commit comments

Comments
 (0)