diff --git a/Makefile b/Makefile index b7db3d235..45fc04393 100644 --- a/Makefile +++ b/Makefile @@ -70,7 +70,6 @@ CCAN_OBJS := \ ccan-crypto-sha256.o \ ccan-crypto-shachain.o \ ccan-crypto-siphash24.o \ - ccan-daemonize.o \ ccan-err.o \ ccan-fdpass.o \ ccan-htable.o \ @@ -132,7 +131,6 @@ CCAN_HEADERS := \ $(CCANDIR)/ccan/crypto/sha256/sha256.h \ $(CCANDIR)/ccan/crypto/shachain/shachain.h \ $(CCANDIR)/ccan/crypto/siphash24/siphash24.h \ - $(CCANDIR)/ccan/daemonize/daemonize.h \ $(CCANDIR)/ccan/endian/endian.h \ $(CCANDIR)/ccan/err/err.h \ $(CCANDIR)/ccan/fdpass/fdpass.h \ @@ -617,8 +615,6 @@ ccan-opt-parse.o: $(CCANDIR)/ccan/opt/parse.c $(CC) $(CFLAGS) -c -o $@ $< ccan-opt-usage.o: $(CCANDIR)/ccan/opt/usage.c $(CC) $(CFLAGS) -c -o $@ $< -ccan-daemonize.o: $(CCANDIR)/ccan/daemonize/daemonize.c - $(CC) $(CFLAGS) -c -o $@ $< ccan-err.o: $(CCANDIR)/ccan/err/err.c $(CC) $(CFLAGS) -c -o $@ $< ccan-noerr.o: $(CCANDIR)/ccan/noerr/noerr.c diff --git a/ccan/ccan/daemonize/LICENSE b/ccan/ccan/daemonize/LICENSE deleted file mode 120000 index 2354d1294..000000000 --- a/ccan/ccan/daemonize/LICENSE +++ /dev/null @@ -1 +0,0 @@ -../../licenses/BSD-MIT \ No newline at end of file diff --git a/ccan/ccan/daemonize/_info b/ccan/ccan/daemonize/_info deleted file mode 100644 index 0c4bc9806..000000000 --- a/ccan/ccan/daemonize/_info +++ /dev/null @@ -1,54 +0,0 @@ -#include "config.h" -#include -#include - -/** - * daemonize - routine to turn a process into a well-behaved daemon. - * - * Daemons should detach themselves thoroughly from the process which launched - * them, and not prevent any filesystems from being unmounted. daemonize() - * helps with the process. - * - * Example: - * #include - * #include - * #include - * #include - * #include - * - * static void usage(const char *name) - * { - * errx(1, "Usage: %s [--daemonize]\n", name); - * } - * - * // Wait for a minute, possibly as a daemon. - * int main(int argc, char *argv[]) - * { - * if (argc != 1) { - * if (argc == 2 && streq(argv[1], "--daemonize")) { - * if (!daemonize()) - * err(1, "Failed to become daemon"); - * } else - * usage(argv[1]); - * } - * sleep(60); - * exit(0); - * } - * - * License: BSD-MIT - */ -int main(int argc, char *argv[]) -{ - if (argc != 2) - return 1; - - if (strcmp(argv[1], "depends") == 0) { - return 0; - } - - if (strcmp(argv[1], "libs") == 0) { - return 0; - } - - return 1; -} diff --git a/ccan/ccan/daemonize/daemonize.c b/ccan/ccan/daemonize/daemonize.c deleted file mode 100644 index bd32ecbba..000000000 --- a/ccan/ccan/daemonize/daemonize.c +++ /dev/null @@ -1,45 +0,0 @@ -/* Licensed under BSD-MIT - see LICENSE file for details */ -#include -#include -#include -#include -#include -#include - -/* This code is based on Stevens Advanced Programming in the UNIX - * Environment. */ -bool daemonize(void) -{ - pid_t pid; - - /* Separate from our parent via fork, so init inherits us. */ - if ((pid = fork()) < 0) - return false; - /* use _exit() to avoid triggering atexit() processing */ - if (pid != 0) - _exit(0); - - /* Don't hold files open. */ - close(STDIN_FILENO); - close(STDOUT_FILENO); - close(STDERR_FILENO); - - /* Many routines write to stderr; that can cause chaos if used - * for something else, so set it here. */ - if (open("/dev/null", O_WRONLY) != 0) - return false; - if (dup2(0, STDERR_FILENO) != STDERR_FILENO) - return false; - close(0); - - /* Session leader so ^C doesn't whack us. */ - if (setsid() == (pid_t)-1) - return false; - /* Move off any mount points we might be in. */ - if (chdir("/") != 0) - return false; - - /* Discard our parent's old-fashioned umask prejudices. */ - umask(0); - return true; -} diff --git a/ccan/ccan/daemonize/daemonize.h b/ccan/ccan/daemonize/daemonize.h deleted file mode 100644 index 1f473eae2..000000000 --- a/ccan/ccan/daemonize/daemonize.h +++ /dev/null @@ -1,21 +0,0 @@ -/* Licensed under BSD-MIT - see LICENSE file for details */ -#ifndef CCAN_DAEMONIZE_H -#define CCAN_DAEMONIZE_H -#include - -/** - * daemonize - turn this process into a daemon. - * - * This routine forks us off to become a daemon. It returns false on failure - * (eg. fork(), chdir or open failed) and sets errno. - * - * Side effects for programmers to be aware of: - * - PID changes (our parent exits, we become child of init) - * - stdin and stdout file descriptors are closed - * - stderr is reopened to /dev/null so you don't reuse it - * - Current working directory changes to / - * - Umask is set to 0. - */ -bool daemonize(void); - -#endif /* CCAN_DAEMONIZE_H */ diff --git a/ccan/ccan/daemonize/test/run.c b/ccan/ccan/daemonize/test/run.c deleted file mode 100644 index 73f021118..000000000 --- a/ccan/ccan/daemonize/test/run.c +++ /dev/null @@ -1,76 +0,0 @@ -#include -#include -#include -#include -#include -#include -#include -#include - -struct child_data { - pid_t pid; - pid_t ppid; - bool in_root_dir; - int read_from_stdin, write_to_stdout, write_to_stderr; -}; - -int main(int argc, char *argv[]) -{ - int fds[2]; - struct child_data daemonized; - pid_t pid; - - plan_tests(5); - - if (pipe(fds) != 0) - err(1, "Failed pipe"); - - /* Since daemonize forks and parent exits, we need to fork - * that parent. */ - pid = fork(); - if (pid == -1) - err(1, "Failed fork"); - - if (pid == 0) { - char buffer[2]; - pid = getpid(); - daemonize(); - /* Keep valgrind happy about uninitialized bytes. */ - memset(&daemonized, 0, sizeof(daemonized)); - daemonized.pid = getpid(); - daemonized.in_root_dir = (getcwd(buffer, 2) != NULL); - daemonized.read_from_stdin - = read(STDIN_FILENO, buffer, 1) == -1 ? errno : 0; - daemonized.write_to_stdout - = write(STDOUT_FILENO, buffer, 1) == -1 ? errno : 0; - if (write(STDERR_FILENO, buffer, 1) != 1) { - daemonized.write_to_stderr = errno; - if (daemonized.write_to_stderr == 0) - daemonized.write_to_stderr = -1; - } else - daemonized.write_to_stderr = 0; - - /* Make sure parent exits. */ - while (getppid() == pid) - sleep(1); - daemonized.ppid = getppid(); - if (write(fds[1], &daemonized, sizeof(daemonized)) - != sizeof(daemonized)) - exit(1); - exit(0); - } - - if (read(fds[0], &daemonized, sizeof(daemonized)) != sizeof(daemonized)) - err(1, "Failed read"); - - ok1(daemonized.pid != pid); -#if 0 /* Believe it or not, this fails under Ubuntu 13.10 (Upstart) */ - ok1(daemonized.ppid == 1); -#endif - ok1(daemonized.in_root_dir); - ok1(daemonized.read_from_stdin == EBADF); - ok1(daemonized.write_to_stdout == EBADF); - ok1(daemonized.write_to_stderr == 0); - - return exit_status(); -} diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index 73cc1d7bf..f29a54ea6 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -42,7 +42,6 @@ #include #include #include -#include #include #include #include @@ -77,6 +76,7 @@ #include #include #include +#include #include #include @@ -198,7 +198,7 @@ static struct lightningd *new_lightningd(const tal_t *ctx) /*~ This is detailed in chaintopology.c */ ld->topology = new_topology(ld, ld->log); - ld->daemon = false; + ld->daemon_parent_fd = -1; ld->config_filename = NULL; ld->pidfile = NULL; ld->proxyaddr = NULL; @@ -501,30 +501,37 @@ static void init_txfilter(struct wallet *w, struct txfilter *filter) * don't prevent unmounting whatever filesystem you happen to start in. * * But we define every path relative to our (~/.lightning) data dir, so we - * make sure we stay there. + * make sure we stay there. The rest of this is taken from ccan/daemonize, + * which was based on W. Richard Steven's advice in Programming in The Unix + * Environment. */ -static void daemonize_but_keep_dir(struct lightningd *ld) +static void complete_daemonize(struct lightningd *ld) { - /* daemonize moves us into /, but we want to be here */ - const char *cwd = path_cwd(NULL); + int ok_status = 0; - /*~ SQLite3 does NOT like being open across fork(), a.k.a. daemonize() */ - db_close_for_fork(ld->wallet->db); - if (!cwd) - fatal("Could not get current directory: %s", strerror(errno)); - if (!daemonize()) - fatal("Could not become a daemon: %s", strerror(errno)); + /* Don't hold files open. */ + close(STDIN_FILENO); + close(STDOUT_FILENO); + close(STDERR_FILENO); - /*~ Move back: important, since lightning dir may be relative! */ - if (chdir(cwd) != 0) - fatal("Could not return to directory %s: %s", - cwd, strerror(errno)); + /* Many routines write to stderr; that can cause chaos if used + * for something else, so set it here. */ + if (open("/dev/null", O_WRONLY) != 0) + fatal("Could not open /dev/null: %s", strerror(errno)); + if (dup2(0, STDERR_FILENO) != STDERR_FILENO) + fatal("Could not dup /dev/null for stderr: %s", strerror(errno)); + close(0); - db_reopen_after_fork(ld->wallet->db); + /* Session leader so ^C doesn't whack us. */ + if (setsid() == (pid_t)-1) + fatal("Could not setsid: %s", strerror(errno)); - /*~ Why not allocate cwd off tmpctx? Probably because this code predates - * tmpctx. So we free manually here. */ - tal_free(cwd); + /* Discard our parent's old-fashioned umask prejudices. */ + umask(0); + + /* OK, parent, you can exit(0) now. */ + write_all(ld->daemon_parent_fd, &ok_status, sizeof(ok_status)); + close(ld->daemon_parent_fd); } /*~ It's pretty standard behaviour (especially for daemons) to create and @@ -785,13 +792,6 @@ int main(int argc, char *argv[]) * in case that runs into trouble. */ crashlog = ld->log; - /*~ We defer --daemon until we've completed most initialization: that - * way we'll exit with an error rather than silently exiting 0, then - * realizing we can't start and forcing the confused user to read the - * logs. */ - if (ld->daemon) - daemonize_but_keep_dir(ld); - /*~ We have to do this after daemonize, since that changes our pid! */ pidfile_write(ld, pid_fd); @@ -830,6 +830,16 @@ int main(int argc, char *argv[]) * can start the poll loop which queries bitcoind for new blocks. */ begin_topology(ld->topology); + /*~ To handle --daemon, we fork the daemon early (otherwise we hit + * issues with our pid changing), but keep the parent around until + * we've completed most initialization: that way we'll exit with an + * error rather than silently exiting 0, then realizing we can't start + * and forcing the confused user to read the logs. + * + * But we're all initialized, so detach and have parent exit now. */ + if (ld->daemon_parent_fd != -1) + complete_daemonize(ld); + /*~ The root of every backtrace (almost). This is our main event * loop. */ void *io_loop_ret = io_loop_with_timers(ld); diff --git a/lightningd/lightningd.h b/lightningd/lightningd.h index ecd8ab72c..d68100739 100644 --- a/lightningd/lightningd.h +++ b/lightningd/lightningd.h @@ -76,8 +76,9 @@ struct lightningd { /* The directory to find all the subdaemons. */ const char *daemon_dir; - /* Are we told to run in the background. */ - bool daemon; + /* If we told to run in the background, this is our parent fd, otherwise + * -1. */ + int daemon_parent_fd; int pid_fd; diff --git a/lightningd/options.c b/lightningd/options.c index a3272f08b..317ea18e0 100644 --- a/lightningd/options.c +++ b/lightningd/options.c @@ -36,6 +36,7 @@ #include #include #include +#include #include #include @@ -721,6 +722,41 @@ static char *opt_lightningd_usage(struct lightningd *ld) return NULL; } +static char *opt_start_daemon(struct lightningd *ld) +{ + int fds[2]; + int exitcode, pid; + + /* Already a daemon? OK. */ + if (ld->daemon_parent_fd != -1) + return NULL; + + if (pipe(fds) != 0) + err(1, "Creating pipe to talk to --daemon"); + + pid = fork(); + if (pid == -1) + err(1, "Fork failed for --daemon"); + + if (pid == 0) { + /* Child returns, continues as normal. */ + close(fds[0]); + ld->daemon_parent_fd = fds[1]; + return NULL; + } + + /* OK, we are the parent. We exit with status told to us by + * child. */ + close(fds[1]); + if (read(fds[0], &exitcode, sizeof(exitcode)) == sizeof(exitcode)) + _exit(exitcode); + /* It died before writing exitcode (presumably 0), so we grab it */ + waitpid(pid, &exitcode, 0); + if (WIFEXITED(exitcode)) + _exit(WEXITSTATUS(exitcode)); + errx(1, "Died with signal %u", WTERMSIG(exitcode)); +} + static char *opt_ignore_talstr(const char *arg, char **p) { return NULL; @@ -809,6 +845,10 @@ static void register_opts(struct lightningd *ld) opt_set_bool_arg, opt_show_bool, &ld->use_proxy_always, "Use the proxy always"); + /* This immediately makes is a daemon. */ + opt_register_early_noarg("--daemon", opt_start_daemon, ld, + "Run in the background, suppress stdout/stderr"); + opt_register_arg("--rpc-file", opt_set_talstr, opt_show_charp, &ld->rpc_filename, "Set JSON-RPC socket (or /dev/tty)"); @@ -847,8 +887,6 @@ static void register_opts(struct lightningd *ld) &ld->pidfile, "Specify pid file"); - opt_register_noarg("--daemon", opt_set_bool, &ld->daemon, - "Run in the background, suppress stdout/stderr"); opt_register_arg("--ignore-fee-limits", opt_set_bool_arg, opt_show_bool, &ld->config.ignore_fee_limits, "(DANGEROUS) allow peer to set any feerate"); @@ -1099,6 +1137,10 @@ static void add_config(struct lightningd *ld, answer = tal_fmt(name0, "%s", (!ld->reconnect && !ld->listen) ? "true" : "false"); + } else if (opt->cb == (void *)opt_start_daemon) { + answer = tal_fmt(name0, "%s", + ld->daemon_parent_fd == -1 + ? "false" : "true"); } else { /* Insert more decodes here! */ assert(!"A noarg option was added but was not handled"); diff --git a/lightningd/test/run-find_my_abspath.c b/lightningd/test/run-find_my_abspath.c index 77abcc44f..649b0f4a7 100644 --- a/lightningd/test/run-find_my_abspath.c +++ b/lightningd/test/run-find_my_abspath.c @@ -51,18 +51,12 @@ void db_assert_no_outstanding_statements(void) /* Generated stub for db_begin_transaction_ */ void db_begin_transaction_(struct db *db UNNEEDED, const char *location UNNEEDED) { fprintf(stderr, "db_begin_transaction_ called!\n"); abort(); } -/* Generated stub for db_close_for_fork */ -void db_close_for_fork(struct db *db UNNEEDED) -{ fprintf(stderr, "db_close_for_fork called!\n"); abort(); } /* Generated stub for db_commit_transaction */ void db_commit_transaction(struct db *db UNNEEDED) { fprintf(stderr, "db_commit_transaction called!\n"); abort(); } /* Generated stub for db_get_intvar */ s64 db_get_intvar(struct db *db UNNEEDED, char *varname UNNEEDED, s64 defval UNNEEDED) { fprintf(stderr, "db_get_intvar called!\n"); abort(); } -/* Generated stub for db_reopen_after_fork */ -void db_reopen_after_fork(struct db *db UNNEEDED) -{ fprintf(stderr, "db_reopen_after_fork called!\n"); abort(); } /* Generated stub for fatal */ void fatal(const char *fmt UNNEEDED, ...) { fprintf(stderr, "fatal called!\n"); abort(); } diff --git a/tests/test_misc.py b/tests/test_misc.py index 7e29b9214..2f9891804 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -659,7 +659,6 @@ def test_cli(node_factory): assert only_one(j['invoices'])['label'] == 'l"[]{}' -@pytest.mark.xfail(strict=True) def test_daemon_option(node_factory): """ Make sure --daemon at least vaguely works! diff --git a/wallet/db.c b/wallet/db.c index c8801e2bb..935f6753d 100644 --- a/wallet/db.c +++ b/wallet/db.c @@ -778,29 +778,6 @@ struct db *db_setup(const tal_t *ctx, struct lightningd *ld, struct log *log) return db; } -void db_close_for_fork(struct db *db) -{ - /* https://www.sqlite.org/faq.html#q6 - * - * Under Unix, you should not carry an open SQLite database across a - * fork() system call into the child process. */ - if (sqlite3_close(db->sql) != SQLITE_OK) - db_fatal("sqlite3_close: %s", sqlite3_errmsg(db->sql)); - db->sql = NULL; -} - -void db_reopen_after_fork(struct db *db) -{ - int err = sqlite3_open_v2(db->filename, &db->sql, - SQLITE_OPEN_READWRITE, NULL); - - if (err != SQLITE_OK) { - db_fatal("failed to re-open database %s: %s", db->filename, - sqlite3_errstr(err)); - } - setup_open_db(db); -} - s64 db_get_intvar(struct db *db, char *varname, s64 defval) { s64 res; diff --git a/wallet/db.h b/wallet/db.h index 45d8c7f3c..52260c5be 100644 --- a/wallet/db.h +++ b/wallet/db.h @@ -149,10 +149,6 @@ void db_stmt_done(sqlite3_stmt *stmt); /* Call when you know there should be no outstanding db statements. */ void db_assert_no_outstanding_statements(void); -/* Do not keep db open across a fork: needed for --daemon */ -void db_close_for_fork(struct db *db); -void db_reopen_after_fork(struct db *db); - #define sqlite3_column_arr(ctx, stmt, col, type) \ ((type *)sqlite3_column_arr_((ctx), (stmt), (col), \ sizeof(type), TAL_LABEL(type, "[]"), \