Merge bitcoin#21007: bitcoind: Add -daemonwait option to wait for ini…

…tialization

e017a91 bitcoind: Add -daemonwait option to wait for initialization (Wladimir J. van der Laan)
c3e6fde shutdown: Use RAII TokenPipe in shutdown (Wladimir J. van der Laan)
612f746 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 bitcoin#21007 (comment).

  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 e017a91 checked change since previous review is move-only

Tree-SHA512: 53369b8ca2247e4cf3af8cb2cfd5b3399e8e0e3296423d64be987004758162a7ddc1287b01a92d7692328edcb2da4cf05d279b1b4ef61a665b71440ab6a6dbe2

build_msvc/bitcoin_config.h

@@ -92,9 +92,9 @@
    don't. */
 #define HAVE_DECL_BSWAP_64 0
 
-/* Define to 1 if you have the declaration of `daemon', and to 0 if you don't.
+/* Define to 1 if you have the declaration of `fork', and to 0 if you don't.
    */
-#define HAVE_DECL_DAEMON 0
+#define HAVE_DECL_FORK 0
 
 /* Define to 1 if you have the declaration of `htobe16', and to 0 if you
    don't. */
@@ -132,6 +132,10 @@
    don't. */
 #define HAVE_DECL_LE64TOH 0
 
+/* Define to 1 if you have the declaration of `setsid', and to 0 if you don't.
+   */
+#define HAVE_DECL_SETSID 0
+
 /* Define to 1 if you have the declaration of `strerror_r', and to 0 if you
    don't. */
 #define HAVE_DECL_STRERROR_R 0

configure.ac

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

src/Makefile.am

@@ -253,6 +253,7 @@ BITCOIN_CORE_H = \
   util/system.h \
   util/threadnames.h \
   util/time.h \
+  util/tokenpipe.h \
   util/trace.h \
   util/translation.h \
   util/ui_change_type.h \
@@ -584,6 +585,7 @@ libbitcoin_util_a_SOURCES = \
   util/strencodings.cpp \
   util/string.cpp \
   util/time.cpp \
+  util/tokenpipe.cpp \
   $(BITCOIN_CORE_H)
 
 if USE_LIBEVENT

src/bitcoind.cpp

@@ -20,6 +20,7 @@
 #include <util/strencodings.h>
 #include <util/system.h>
 #include <util/threadnames.h>
+#include <util/tokenpipe.h>
 #include <util/translation.h>
 #include <util/url.h>
 
@@ -28,6 +29,79 @@
 const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
 UrlDecodeFn* const URL_DECODE = urlDecode;
 
+#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;
@@ -59,6 +133,14 @@ static bool AppInit(int argc, char* argv[])
         return true;
     }
 
+#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
     util::Ref context{node};
     try
     {
@@ -105,24 +187,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 HAVE_DECL_DAEMON
-#if defined(MAC_OSX)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-#endif
+        if (args.GetBoolArg("-daemon", DEFAULT_DAEMON) || args.GetBoolArg("-daemonwait", DEFAULT_DAEMONWAIT)) {
+#if HAVE_DECL_FORK
             tfm::format(std::cout, PACKAGE_NAME " starting\n");
 
             // Daemonize
-            if (daemon(1, 0)) { // don't chdir (1), do close FDs (0)
-                return InitError(Untranslated(strprintf("daemon() failed: %s\n", strerror(errno))));
+            switch (fork_daemon(1, 0, daemon_ep)) { // don't chdir (1), do close FDs (0)
+            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())
@@ -138,6 +230,13 @@ static bool AppInit(int argc, char* argv[])
         PrintExceptionContinue(nullptr, "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

@@ -580,8 +580,9 @@ void SetupServerArgs(NodeContext& node)
     argsman.AddArg("-rpcworkqueue=<n>", strprintf("Set the depth of the work queue to service RPC calls (default: %d)", DEFAULT_HTTP_WORKQUEUE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::RPC);
     argsman.AddArg("-server", "Accept command line and JSON-RPC commands", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
 
-#if HAVE_DECL_DAEMON
-    argsman.AddArg("-daemon", "Run in the background as a daemon and accept commands", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
+#if HAVE_DECL_FORK
+    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

@@ -9,6 +9,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

@@ -5,16 +5,15 @@
 
 #include <shutdown.h>
 
+#include <logging.h>
+#include <util/tokenpipe.h>
+
 #include <config/bitcoin-config.h>
 
 #include <assert.h>
 #include <atomic>
 #ifdef WIN32
 #include <condition_variable>
-#else
-#include <errno.h>
-#include <fcntl.h>
-#include <unistd.h>
 #endif
 
 static std::atomic<bool> fRequestShutdown(false);
@@ -24,25 +23,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
-    // If we can, make sure that the file descriptors are closed on exec()
-    // to prevent interference.
-    if (pipe2(g_shutdown_pipe, O_CLOEXEC) != 0) {
-        return false;
-    }
-#else
-    if (pipe(g_shutdown_pipe) != 0) {
-        return false;
-    }
-#endif
+    std::optional<TokenPipe> pipe = TokenPipe::Make();
+    if (!pipe) return false;
+    g_shutdown_r = pipe->TakeReadEnd();
+    g_shutdown_w = pipe->TakeWriteEnd();
 #endif
     return true;
 }
@@ -59,17 +51,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';
-        while (true) {
-            int result = write(g_shutdown_pipe[1], &token, 1);
-            if (result < 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;
-            }
+        int res = g_shutdown_w.TokenWrite('x');
+        if (res != 0) {
+            LogPrintf("Sending shutdown token failed\n");
+            assert(0);
         }
     }
 #endif
@@ -96,17 +81,10 @@ void WaitForShutdown()
     std::unique_lock<std::mutex> lk(g_shutdown_mutex);
     g_shutdown_cv.wait(lk, [] { return fRequestShutdown.load(); });
 #else
-    char token;
-    while (true) {
-        int result = read(g_shutdown_pipe[0], &token, 1);
-        if (result < 0) {
-            // Failure. Check if the read was interrupted by a signal.
-            // Other errors are unexpected here.
-            assert(errno == EINTR);
-        } else {
-            assert(result == 1);
-            break;
-        }
+    int res = g_shutdown_r.TokenRead();
+    if (res != 'x') {
+        LogPrintf("Reading shutdown token failed\n");
+        assert(0);
     }
 #endif
 }