Forking daemon

[ChoHag]

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.

[laanwj]

Concept ACK

[sipa]

Concept ACK

[laanwj]

Nice use of the build system

[jonasschnelli]

Concept ACK

[ChoHag]

I'm not overly concerned with making the feature freeze but I realised it's a bad code smell to have two mutually exclusive command line arguments, so have simply removed the chdir for the time being.

[laanwj]

Thanks. Will test.

[laanwj]

I get the following compile error when I force HAVE_DECL_DAEMON off:

/home/user/src/bitcoin/src/bitcoind.cpp:164:27: error: use of undeclared identifier '_PATH_DEVNULL'
            int fd = open(_PATH_DEVNULL, 02, 0);

Where is that definition supposed to come from?

[ChoHag]

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.

[sipa]

Anything holding this up?

[laanwj]

Yes, it's not compiling for me when pretending not to have daemonize(). If we can't test that fallback code, we shouldn't even include it.
@ChoHag was working on a change.
(probably an autotools detection for _PATH_DEVNULL)

[laanwj]

Note that we do have some things that potentially print to stdout/stderr after daemonize():

• util.cpp: PrintExceptionContinue
• noui.cpp: noui_ThreadSafeMessageBox (used for reporting some errors during http initialization)
• sync.cpp: AssertLockHeldInternal

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.

[gmaxwell]

@ChoHag Around? If not then maybe someone else wants to pick this up?

[ChoHag]

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)

[laanwj]

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.

[laanwj]

Closing in favor of #8813