When started with cron, the sendmail process hung around waiting for its stdin, bitcoind’s stdout/err, to close. Fix that using daemon(3) where possible and doing its work where not.
Also makes the forking error messages more consistent and, by using strerror, more explanatory.
[bitcoind] Make forking error messages consistent
513@@ -514,6 +514,9 @@ AC_SEARCH_LIBS([inet_pton], [nsl resolv], [AC_DEFINE(HAVE_INET_PTON, 1, [Define
514
515 AC_CHECK_DECLS([strnlen])
516
517+# Check for daemon(3), unrelated to --with-daemon (although used by it)
518+AC_CHECK_DECLS([daemon])
158@@ -150,9 +159,20 @@ bool AppInit(int argc, char* argv[])
159
160 pid_t sid = setsid();
161 if (sid < 0)
162- fprintf(stderr, "Error: setsid() returned %d errno %d\n", sid, errno);
163+ fprintf(stderr, "Error: setsid() failed: %s\n", strerror(errno));
164+
165+ (void)chdir("/");
-datadir, -conf, but also the -*notify options) that are affected by this.
Having looked up alternative ways of dealing with absolute/relative paths, I’ve come up with these options:
There are advantages to performing the chdir to / when daemonising especially if/when bitcoind becomes suitable to use as a system service so if it can be done with minimal fuss I think it should be.
I like the chdir, myself, and think it will look neater than ‘cd / && bitcoind’ so may I propose two new command-line switches:
I considered giving -chdir an optional option to specify which directory, with / or C:\ as the default, but that seems like overkill and makes option parsing Magic; a dirty word.
I think this fits well with bitcoind’s triple life as a system daemon, user daemon or user interface.
Edit: One more thing, I’m aware that in the grand scheme of things this addition is largely unimportant and have no intention of starting a bike-shedding.
[*] Not what it really stands for.
daemonize that keeps the behavior the same, it’s much easier to get it merged, even for 0.13.
If you start adding command-line flags this is too late for the feature freeze, so will only be considered after the 0.14 branch (and you will have to document the changes in the release notes).
I get the following compile error when I force HAVE_DECL_DAEMON off:
0/home/user/src/bitcoin/src/bitcoind.cpp:164:27: error: use of undeclared identifier '_PATH_DEVNULL'
1 int fd = open(_PATH_DEVNULL, 02, 0);
Where is that definition supposed to come from?
It’s from /usr/include/paths.h but now that I see the initial _ (and, of course, your error) I wonder how portable it is. The file’s there on OpenBSD and Linux. I don’t have anything else to hand to look in but notably the file’s copyright is 1993 to Berkely so if not officially portable it’s probably practically portable.
According to gnulib’s manual: Portability problems not fixed by Gnulib: This header file is missing on some platforms: Minix 3.1.8, HP-UX 11, Solaris 11 2010-11, mingw, MSVC 9, BeOS. The set of PATH* macros is platform dependent.
I’ll push a change shortly to be more thorough.
Note that we do have some things that potentially print to stdout/stderr after daemonize():
Should not hold up this pull, as this is bad behavior in the first place, but we should make sure that everything printed also ends up in the log.
I’m not really, I have pressing personal matters to deal with and cannot devote time to hacking so I’m happy to hand this over to someone else. I was researching the best way to make the change fully portable but really the whole process is quite simple.
Matthew
@ChoHag Around? If not then maybe someone else wants to pick this up?
You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #8278 (comment)
Are there any POSIX-ish OSes that are supported by our build that don’t support daemon()? I’ve checked at least Linux and OpenBSD and they do. OSX does too (thanks @jonasschnelli).
If not, we could just disable -daemon if that call is not available (like on WIndows). That makes it a lot easier to do this as well as maintain the code afterward as the fallback path is not needed.