Merge #21007: bitcoind: Add -daemonwait option to wait for initialization

e017a913d0d78ef0766cf73586fe7a38488e1a26 bitcoind: Add -daemonwait option to wait for initialization (Wladimir J. van der Laan)
c3e6fdee6d39d3f52dec421b48a0ac8bad5006f7 shutdown: Use RAII TokenPipe in shutdown (Wladimir J. van der Laan)
612f746a8ffa265b6877bedbbe21fcbb392f1516 util: Add RAII TokenPipe (Wladimir J. van der Laan)

Pull request description:

  This adds a `-daemonwait` flag that does the same as `-daemon` except that it, from a user perspective, backgrounds the process only after initialization is complete. This is similar to the behaviour of some other software such as c-lightning.

  This can be useful when the process launching bitcoind wants to guarantee that either the RPC server is running, or that initialization failed, before continuing. The exit code indicates the initialization result.

  The use of the libc function `daemon()` is replaced by a custom implementation which is inspired by the [glibc implementation](https://github.com/lattera/glibc/blob/master/misc/daemon.c#L44), but which also creates a pipe from the child to the parent process for communication.

  An additional advantage of having our own `daemon()` implementation is that no MACOS-specific pragmas are needed anymore to silence a deprecation warning.

  TODO:

  - [x] Factor out `token_read` and `token_write` to an utility, and use  them in `shutdown.cpp` as well—this is exactly the same kind of communication mechanism.

      - [x] RAII-ify pipe endpoints.

  - [x] Improve granularity of the `configure.ac` checks. This currently  still checks for the function `daemon()` which makes no sense as  it's not used. It should check for individual functions such as
    `fork()` and `setsid()` etc—the former being required, the second optional.

  - [-] ~~Signal propagation during initialization: if say, pressing Ctrl-C during `-daemonwait` it would be good to pass this SIGINT on to the child process instead of detaching the parent process and letting the child run free.~~ This is not necessary, see https://github.com/bitcoin/bitcoin/pull/21007#issuecomment-769007341.

  Future:

  - Consider if it makes sense to use this in the RPC tests (there would be no more need for "is RPC ready" polling loops). I think this is out of scope for this PR.

ACKs for top commit:
  jonatack:
    Tested ACK e017a913d0d78ef0766cf73586fe7a38488e1a26 checked change since previous review is move-only

Tree-SHA512: 53369b8ca2247e4cf3af8cb2cfd5b3399e8e0e3296423d64be987004758162a7ddc1287b01a92d7692328edcb2da4cf05d279b1b4ef61a665b71440ab6a6dbe2

configure.ac

@@ -1007,8 +1007,9 @@ AC_CHECK_DECLS([getifaddrs, freeifaddrs],[CHECK_SOCKET],,
 )
 AC_CHECK_DECLS([strnlen])
 
-dnl Check for daemon(3), unrelated to --with-daemon (although used by it)
+dnl These are used for daemonization in dashd
-AC_CHECK_DECLS([daemon])
+AC_CHECK_DECLS([fork])
+AC_CHECK_DECLS([setsid])
 
 AC_CHECK_DECLS([pipe2])
 

src/Makefile.am

@@ -355,6 +355,7 @@ BITCOIN_CORE_H = \
   util/time.h \
   util/thread.h \
   util/threadnames.h \
+  util/tokenpipe.h \
   util/trace.h \
   util/translation.h \
   util/ui_change_type.h \
@@ -792,6 +793,7 @@ libbitcoin_util_a_SOURCES = \
   util/string.cpp \
   util/thread.cpp \
   util/threadnames.cpp \
+  util/tokenpipe.cpp \
   $(BITCOIN_CORE_H)
 
 if USE_LIBEVENT

src/bitcoind.cpp

@@ -21,6 +21,7 @@
 #include <util/system.h>
 #include <util/strencodings.h>
 #include <util/threadnames.h>
+#include <util/tokenpipe.h>
 #include <util/translation.h>
 #include <stacktraces.h>
 #include <util/url.h>
@@ -35,6 +36,79 @@ UrlDecodeFn* const URL_DECODE = urlDecode;
 //
 // Start
 //
+#if HAVE_DECL_FORK
+
+/** Custom implementation of daemon(). This implements the same order of operations as glibc.
+ * Opens a pipe to the child process to be able to wait for an event to occur.
+ *
+ * @returns 0 if successful, and in child process.
+ *          >0 if successful, and in parent process.
+ *          -1 in case of error (in parent process).
+ *
+ *          In case of success, endpoint will be one end of a pipe from the child to parent process,
+ *          which can be used with TokenWrite (in the child) or TokenRead (in the parent).
+ */
+int fork_daemon(bool nochdir, bool noclose, TokenPipeEnd& endpoint)
+{
+    // communication pipe with child process
+    std::optional<TokenPipe> umbilical = TokenPipe::Make();
+    if (!umbilical) {
+        return -1; // pipe or pipe2 failed.
+    }
+
+    int pid = fork();
+    if (pid < 0) {
+        return -1; // fork failed.
+    }
+    if (pid != 0) {
+        // Parent process gets read end, closes write end.
+        endpoint = umbilical->TakeReadEnd();
+        umbilical->TakeWriteEnd().Close();
+
+        int status = endpoint.TokenRead();
+        if (status != 0) { // Something went wrong while setting up child process.
+            endpoint.Close();
+            return -1;
+        }
+
+        return pid;
+    }
+    // Child process gets write end, closes read end.
+    endpoint = umbilical->TakeWriteEnd();
+    umbilical->TakeReadEnd().Close();
+
+#if HAVE_DECL_SETSID
+    if (setsid() < 0) {
+        exit(1); // setsid failed.
+    }
+#endif
+
+    if (!nochdir) {
+        if (chdir("/") != 0) {
+            exit(1); // chdir failed.
+        }
+    }
+    if (!noclose) {
+        // Open /dev/null, and clone it into STDIN, STDOUT and STDERR to detach
+        // from terminal.
+        int fd = open("/dev/null", O_RDWR);
+        if (fd >= 0) {
+            bool err = dup2(fd, STDIN_FILENO) < 0 || dup2(fd, STDOUT_FILENO) < 0 || dup2(fd, STDERR_FILENO) < 0;
+            // Don't close if fd<=2 to try to handle the case where the program was invoked without any file descriptors open.
+            if (fd > 2) close(fd);
+            if (err) {
+                exit(1); // dup2 failed.
+            }
+        } else {
+            exit(1); // open /dev/null failed.
+        }
+    }
+    endpoint.TokenWrite(0); // Success
+    return 0;
+}
+
+#endif
+
 static bool AppInit(int argc, char* argv[])
 {
     NodeContext node;
@@ -72,6 +146,14 @@ static bool AppInit(int argc, char* argv[])
     }
 
     CoreContext context{node};
+#if HAVE_DECL_FORK
+    // Communication with parent after daemonizing. This is used for signalling in the following ways:
+    // - a boolean token is sent when the initialization process (all the Init* functions) have finished to indicate
+    // that the parent process can quit, and whether it was successful/unsuccessful.
+    // - an unexpected shutdown of the child process creates an unexpected end of stream at the parent
+    // end, which is interpreted as failure to start.
+    TokenPipeEnd daemon_ep;
+#endif
     try
     {
         if (!CheckDataDirOption()) {
@@ -117,24 +199,34 @@ static bool AppInit(int argc, char* argv[])
             // InitError will have been called with detailed error, which ends up on console
             return false;
         }
-        if (args.GetBoolArg("-daemon", false)) {
+        if (args.GetBoolArg("-daemon", DEFAULT_DAEMON) || args.GetBoolArg("-daemonwait", DEFAULT_DAEMONWAIT)) {
-#if HAVE_DECL_DAEMON
+#if HAVE_DECL_FORK
-#if defined(MAC_OSX)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-#endif
             tfm::format(std::cout, PACKAGE_NAME " starting\n");
 
             // Daemonize
-            if (daemon(1, 0)) { // don't chdir (1), do close FDs (0)
+            switch (fork_daemon(1, 0, daemon_ep)) { // don't chdir (1), do close FDs (0)
-                return InitError(Untranslated(strprintf("daemon() failed: %s\n", strerror(errno))));
+            case 0: // Child: continue.
+                // If -daemonwait is not enabled, immediately send a success token the parent.
+                if (!args.GetBoolArg("-daemonwait", DEFAULT_DAEMONWAIT)) {
+                    daemon_ep.TokenWrite(1);
+                    daemon_ep.Close();
+                }
+                break;
+            case -1: // Error happened.
+                return InitError(Untranslated(strprintf("fork_daemon() failed: %s\n", strerror(errno))));
+            default: { // Parent: wait and exit.
+                int token = daemon_ep.TokenRead();
+                if (token) { // Success
+                    exit(EXIT_SUCCESS);
+                } else { // fRet = false or token read error (premature exit).
+                    tfm::format(std::cerr, "Error during initializaton - check debug.log for details\n");
+                    exit(EXIT_FAILURE);
+                }
+            }
             }
-#if defined(MAC_OSX)
-#pragma GCC diagnostic pop
-#endif
 #else
             return InitError(Untranslated("-daemon is not supported on this operating system\n"));
-#endif // HAVE_DECL_DAEMON
+#endif // HAVE_DECL_FORK
         }
         // Lock data directory after daemonization
         if (!AppInitLockDataDirectory())
@@ -147,6 +239,13 @@ static bool AppInit(int argc, char* argv[])
         PrintExceptionContinue(std::current_exception(), "AppInit()");
     }
 
+#if HAVE_DECL_FORK
+    if (daemon_ep.IsOpen()) {
+        // Signal initialization status to parent, then close pipe.
+        daemon_ep.TokenWrite(fRet);
+        daemon_ep.Close();
+    }
+#endif
     if (fRet) {
         WaitForShutdown();
     }

src/init.cpp

@@ -779,8 +779,9 @@ void SetupServerArgs(NodeContext& node)
     argsman.AddArg("-statsport=<port>", strprintf("Specify statsd port (default: %u)", DEFAULT_STATSD_PORT), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD);
     argsman.AddArg("-statsns=<ns>", strprintf("Specify additional namespace prefix (default: %s)", DEFAULT_STATSD_NAMESPACE), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD);
     argsman.AddArg("-statsperiod=<seconds>", strprintf("Specify the number of seconds between periodic measurements (default: %d)", DEFAULT_STATSD_PERIOD), ArgsManager::ALLOW_ANY, OptionsCategory::STATSD);
-#if HAVE_DECL_DAEMON
+#if HAVE_DECL_FORK
-    argsman.AddArg("-daemon", "Run in the background as a daemon and accept commands", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+    argsman.AddArg("-daemon", strprintf("Run in the background as a daemon and accept commands (default: %d)", DEFAULT_DAEMON), ArgsManager::ALLOW_BOOL, OptionsCategory::OPTIONS);
+    argsman.AddArg("-daemonwait", strprintf("Wait for initialization to be finished before exiting. This implies -daemon (default: %d)", DEFAULT_DAEMONWAIT), ArgsManager::ALLOW_BOOL, OptionsCategory::OPTIONS);
 #else
     hidden_args.emplace_back("-daemon");
 #endif

src/init.h

@@ -11,6 +11,11 @@
 #include <memory>
 #include <string>
 
+//! Default value for -daemon option
+static constexpr bool DEFAULT_DAEMON = false;
+//! Default value for -daemonwait option
+static constexpr bool DEFAULT_DAEMONWAIT = false;
+
 class ArgsManager;
 struct NodeContext;
 namespace interfaces {

src/shutdown.cpp

@@ -6,6 +6,8 @@
 #include <shutdown.h>
 
 #include <logging.h>
+#include <util/tokenpipe.h>
+
 #include <node/ui_interface.h>
 #include <warnings.h>
 
@@ -15,10 +17,6 @@
 #include <atomic>
 #ifdef WIN32
 #include <condition_variable>
-#else
-#include <errno.h>
-#include <fcntl.h>
-#include <unistd.h>
 #endif
 
 bool AbortNode(const std::string& strMessage, bilingual_str user_message)
@@ -42,25 +40,18 @@ std::mutex g_shutdown_mutex;
 std::condition_variable g_shutdown_cv;
 #else
 /** On UNIX-like operating systems use the self-pipe trick.
- * Index 0 will be the read end of the pipe, index 1 the write end.
  */
-static int g_shutdown_pipe[2] = {-1, -1};
+static TokenPipeEnd g_shutdown_r;
+static TokenPipeEnd g_shutdown_w;
 #endif
 
 bool InitShutdownState()
 {
 #ifndef WIN32
-#if HAVE_O_CLOEXEC && HAVE_DECL_PIPE2
+    std::optional<TokenPipe> pipe = TokenPipe::Make();
-    // If we can, make sure that the file descriptors are closed on exec()
+    if (!pipe) return false;
-    // to prevent interference.
+    g_shutdown_r = pipe->TakeReadEnd();
-    if (pipe2(g_shutdown_pipe, O_CLOEXEC) != 0) {
+    g_shutdown_w = pipe->TakeWriteEnd();
-        return false;
-    }
-#else
-    if (pipe(g_shutdown_pipe) != 0) {
-        return false;
-    }
-#endif
 #endif
     return true;
 }
@@ -77,17 +68,10 @@ void StartShutdown()
     // case of a reentrant signal.
     if (!fRequestShutdown.exchange(true)) {
         // Write an arbitrary byte to the write end of the shutdown pipe.
-        const char token = 'x';
+        int res = g_shutdown_w.TokenWrite('x');
-        while (true) {
+        if (res != 0) {
-            int result = write(g_shutdown_pipe[1], &token, 1);
+            LogPrintf("Sending shutdown token failed\n");
-            if (result < 0) {
+            assert(0);
-                // Failure. It's possible that the write was interrupted by another signal.
-                // Other errors are unexpected here.
-                assert(errno == EINTR);
-            } else {
-                assert(result == 1);
-                break;
-            }
         }
     }
 #endif
@@ -124,17 +108,10 @@ void WaitForShutdown()
     std::unique_lock<std::mutex> lk(g_shutdown_mutex);
     g_shutdown_cv.wait(lk, [] { return fRequestShutdown.load(); });
 #else
-    char token;
+    int res = g_shutdown_r.TokenRead();
-    while (true) {
+    if (res != 'x') {
-        int result = read(g_shutdown_pipe[0], &token, 1);
+        LogPrintf("Reading shutdown token failed\n");
-        if (result < 0) {
+        assert(0);
-            // Failure. Check if the read was interrupted by a signal.
-            // Other errors are unexpected here.
-            assert(errno == EINTR);
-        } else {
-            assert(result == 1);
-            break;
-        }
     }
 #endif
 }

src/util/tokenpipe.cpp

@@ -0,0 +1,108 @@
+// Copyright (c) 2021 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+#include <util/tokenpipe.h>
+
+#include <config/bitcoin-config.h>
+
+#ifndef WIN32
+
+#include <errno.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+TokenPipeEnd TokenPipe::TakeReadEnd()
+{
+    TokenPipeEnd res(m_fds[0]);
+    m_fds[0] = -1;
+    return res;
+}
+
+TokenPipeEnd TokenPipe::TakeWriteEnd()
+{
+    TokenPipeEnd res(m_fds[1]);
+    m_fds[1] = -1;
+    return res;
+}
+
+TokenPipeEnd::TokenPipeEnd(int fd) : m_fd(fd)
+{
+}
+
+TokenPipeEnd::~TokenPipeEnd()
+{
+    Close();
+}
+
+int TokenPipeEnd::TokenWrite(uint8_t token)
+{
+    while (true) {
+        ssize_t result = write(m_fd, &token, 1);
+        if (result < 0) {
+            // Failure. It's possible that the write was interrupted by a signal,
+            // in that case retry.
+            if (errno != EINTR) {
+                return TS_ERR;
+            }
+        } else if (result == 0) {
+            return TS_EOS;
+        } else { // ==1
+            return 0;
+        }
+    }
+}
+
+int TokenPipeEnd::TokenRead()
+{
+    uint8_t token;
+    while (true) {
+        ssize_t result = read(m_fd, &token, 1);
+        if (result < 0) {
+            // Failure. Check if the read was interrupted by a signal,
+            // in that case retry.
+            if (errno != EINTR) {
+                return TS_ERR;
+            }
+        } else if (result == 0) {
+            return TS_EOS;
+        } else { // ==1
+            return token;
+        }
+    }
+    return token;
+}
+
+void TokenPipeEnd::Close()
+{
+    if (m_fd != -1) close(m_fd);
+    m_fd = -1;
+}
+
+std::optional<TokenPipe> TokenPipe::Make()
+{
+    int fds[2] = {-1, -1};
+#if HAVE_O_CLOEXEC && HAVE_DECL_PIPE2
+    if (pipe2(fds, O_CLOEXEC) != 0) {
+        return std::nullopt;
+    }
+#else
+    if (pipe(fds) != 0) {
+        return std::nullopt;
+    }
+#endif
+    return TokenPipe(fds);
+}
+
+TokenPipe::~TokenPipe()
+{
+    Close();
+}
+
+void TokenPipe::Close()
+{
+    if (m_fds[0] != -1) close(m_fds[0]);
+    if (m_fds[1] != -1) close(m_fds[1]);
+    m_fds[0] = m_fds[1] = -1;
+}
+
+#endif // WIN32

src/util/tokenpipe.h

@@ -0,0 +1,127 @@
+// Copyright (c) 2021 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_UTIL_TOKENPIPE_H
+#define BITCOIN_UTIL_TOKENPIPE_H
+
+#ifndef WIN32
+
+#include <cstdint>
+#include <optional>
+
+/** One end of a token pipe. */
+class TokenPipeEnd
+{
+private:
+    int m_fd = -1;
+
+public:
+    TokenPipeEnd(int fd = -1);
+    ~TokenPipeEnd();
+
+    /** Return value constants for TokenWrite and TokenRead. */
+    enum Status {
+        TS_ERR = -1, //!< I/O error
+        TS_EOS = -2, //!< Unexpected end of stream
+    };
+
+    /** Write token to endpoint.
+     *
+     * @returns 0       If successful.
+     *          <0 if error:
+     *            TS_ERR  If an error happened.
+     *            TS_EOS  If end of stream happened.
+     */
+    int TokenWrite(uint8_t token);
+
+    /** Read token from endpoint.
+     *
+     * @returns >=0     Token value, if successful.
+     *          <0 if error:
+     *            TS_ERR  If an error happened.
+     *            TS_EOS  If end of stream happened.
+     */
+    int TokenRead();
+
+    /** Explicit close function.
+     */
+    void Close();
+
+    /** Return whether endpoint is open.
+     */
+    bool IsOpen() { return m_fd != -1; }
+
+    // Move-only class.
+    TokenPipeEnd(TokenPipeEnd&& other)
+    {
+        m_fd = other.m_fd;
+        other.m_fd = -1;
+    }
+    TokenPipeEnd& operator=(TokenPipeEnd&& other)
+    {
+        Close();
+        m_fd = other.m_fd;
+        other.m_fd = -1;
+        return *this;
+    }
+    TokenPipeEnd(const TokenPipeEnd&) = delete;
+    TokenPipeEnd& operator=(const TokenPipeEnd&) = delete;
+};
+
+/** An interprocess or interthread pipe for sending tokens (one-byte values)
+ * over.
+ */
+class TokenPipe
+{
+private:
+    int m_fds[2] = {-1, -1};
+
+    TokenPipe(int fds[2]) : m_fds{fds[0], fds[1]} {}
+
+public:
+    ~TokenPipe();
+
+    /** Create a new pipe.
+     * @returns The created TokenPipe, or an empty std::nullopt in case of error.
+     */
+    static std::optional<TokenPipe> Make();
+
+    /** Take the read end of this pipe. This can only be called once,
+     * as the object will be moved out.
+     */
+    TokenPipeEnd TakeReadEnd();
+
+    /** Take the write end of this pipe. This should only be called once,
+     * as the object will be moved out.
+     */
+    TokenPipeEnd TakeWriteEnd();
+
+    /** Close and end of the pipe that hasn't been moved out.
+     */
+    void Close();
+
+    // Move-only class.
+    TokenPipe(TokenPipe&& other)
+    {
+        for (int i = 0; i < 2; ++i) {
+            m_fds[i] = other.m_fds[i];
+            other.m_fds[i] = -1;
+        }
+    }
+    TokenPipe& operator=(TokenPipe&& other)
+    {
+        Close();
+        for (int i = 0; i < 2; ++i) {
+            m_fds[i] = other.m_fds[i];
+            other.m_fds[i] = -1;
+        }
+        return *this;
+    }
+    TokenPipe(const TokenPipe&) = delete;
+    TokenPipe& operator=(const TokenPipe&) = delete;
+};
+
+#endif // WIN32
+
+#endif // BITCOIN_UTIL_TOKENPIPE_H