bitcoind: Daemonize using daemon(3) #8813

pull laanwj wants to merge 1 commits into bitcoin:master from laanwj:2016_09_daemonize changing 3 files +12 −16

laanwj commented at 9:39 AM on September 26, 2016: member
Simplified version of #8278. Assumes that every OS that (a) is supported by Bitcoin Core (b) supports daemonization has the daemon() function in its C library.
- Removes the fallback path for operating systems that support daemonization but not daemon(). This prevents never-exercised code from ending up in the repository (see discussion here: #8278 (comment)).
- Removes the windows-specific path. Windows doesn't support daemon(), so it doesn't support daemonization there, automatically. Give an explicit error if a user specifies -daemon on an OS where this is not supported.
- Also made showing the help message depend on HAVE_DECL_DAEMON instead of !WIN32.
The original problem reported in #8278 was "When started with cron, the sendmail process hung around waiting for its stdin, bitcoind's stdout/err, to close..".
laanwj added the label Utils and libraries on Sep 26, 2016
laanwj cross-referenced this on Sep 26, 2016 from issue Forking daemon by ChoHag

in src/bitcoind.cpp:None in 365c2d9fea outdated

 137 |              // Daemonize
 138 | -            pid_t pid = fork();
 139 | -            if (pid < 0)
 140 | -            {
 141 | -                fprintf(stderr, "Error: fork() returned %d errno %d\n", pid, errno);
 142 | +            // daemon(3) basically does exactly what follows in the else block,

sipa commented at 9:44 AM on September 26, 2016:

That's not true anymore.

laanwj commented at 9:45 AM on September 26, 2016:

Two more lines that can be deleted, thanks.

laanwj force-pushed on Sep 26, 2016
laanwj force-pushed on Sep 26, 2016
laanwj force-pushed on Sep 26, 2016
laanwj renamed this:
~~bitcoind: Daemonise using daemon(3)~~
bitcoind: Daemonize using daemon(3)
on Sep 26, 2016

bitcoind: Daemonize using daemon(3)

Simplified version of #8278. Assumes that every OS that (a) is supported
by Bitcoin Core (b) supports daemonization has the `daemon()` function
in its C library.

- Removes the fallback path for operating systems that support
  daemonization but not `daemon()`. This prevents never-exercised code from
  ending up in the repository (see discussion here:
  https://github.com/bitcoin/bitcoin/pull/8278#issuecomment-242704745).

- Removes the windows-specific path. Windows doesn't support `daemon()`,
  so it don't support daemonization there, automatically.

Original code by Matthew King, adapted by Wladimir van der Laan.

a92bf4af66

laanwj force-pushed on Sep 26, 2016
jonasschnelli approved
jonasschnelli commented at 11:53 AM on September 26, 2016: contributor

utACK https://github.com/bitcoin/bitcoin/pull/8813/commits/a92bf4af66f8b5a2e6bc28baa6670065ce76daf2 (-comment nit mentioned by @sipa)
jonasschnelli commented at 2:10 PM on September 26, 2016: contributor

Tested ACK (OSX) https://github.com/bitcoin/bitcoin/pull/8813/commits/a92bf4af66f8b5a2e6bc28baa6670065ce76daf2. Binaries: https://bitcoin.jonasschnelli.ch/pulls/8813/

<img width="498" alt="bildschirmfoto 2016-09-26 um 16 08 46" src="https://cloud.githubusercontent.com/assets/178464/18837302/943ae81a-8403-11e6-9f1d-fa34e16d741c.png">
MarcoFalke commented at 8:22 PM on September 27, 2016: member

Possibly related: #4676 (haven't checked)
sipa commented at 12:20 AM on September 28, 2016: member

Concept ACK.
laanwj commented at 7:18 AM on September 28, 2016: member

Possibly related: #4676 (haven't checked)

Yes, using daemonize() seems quite a step closer to standard UNIX daemon practices. Though we had to decide in #8278 to use the nochdir flag as, without further changes, it would have effects on option behavior.
laanwj merged this on Sep 30, 2016
laanwj closed this on Sep 30, 2016
laanwj referenced this in commit fb24d7eeb4 on Sep 30, 2016
laanwj commented at 4:21 PM on September 30, 2016: member

Note: if you end up here seeing the "Error: -daemon is not supported on this operating system" message while your OS does support it, this most likely means you need to re-run autogen.sh and configure.

paveljanik commented at 8:59 PM on October 21, 2016: contributor

This brings new warning on OS X:

bitcoind.cpp:137:17: warning: 'daemon' is deprecated: first deprecated in OS X 10.5 [-Wdeprecated-declarations]
            if (daemon(1, 0)) {
                ^
/usr/include/stdlib.h:267:5: note: 'daemon' has been explicitly marked deprecated here
int daemon(int, int) __asm("_" "daemon" "$1050") __attribute__((availability(macosx,introduced=10.0,deprecated=10.5)));
    ^
1 warning generated.

laanwj commented at 7:27 AM on October 22, 2016: member

You can't be friggin serious. I'm going to revert this - it's only causing trouble.
laanwj commented at 7:27 AM on October 22, 2016: member

@paveljanik Just curious do they suggest anything to use instead of daemon?
jonasschnelli commented at 7:43 AM on October 22, 2016: contributor

I wouldn't take the deprecation warning to serious. OSX want devs to use its own launchd/launchctrl. Removing the actual daemon() systemcall would break tons of applications and if - we would have at least 1-2 years to switch to launchd on OSX.
laanwj commented at 7:43 AM on October 22, 2016: member

Apparently they recommend doing something Apple-specific with plists and such https://developer.apple.com/library/content/technotes/tn2083/_index.html#//%20apple_ref/doc/uid/DTS10003794-CH1-SUBSECTION64 Bah. Okay, reverting this isn't any help either then. Never mind that. I got upset.
paveljanik commented at 8:15 AM on October 22, 2016: contributor

I agree with @jonasschnelli. No need to revert this. This only shows how Apple deprecates stuff. 10.5. Deprecated since October 26, 2007, still works now ;-)

What is the current status of daemon in the current macOS?
laanwj cross-referenced this on Oct 24, 2016 from issue OS-specific background service for bitcoind by laanwj
laanwj cross-referenced this on Dec 6, 2016 from issue Suppress some annoying deprecation warnings (OSX) by jonasschnelli
luke-jr referenced this in commit 377ab84465 on Dec 21, 2016
sipa cross-referenced this on Mar 31, 2017 from issue bitcoind ignores -printtoconsole flag by UniversalBinary
codablock referenced this in commit e9bf59fb07 on Sep 19, 2017
codablock referenced this in commit 210242e5c0 on Jan 12, 2018
Sjors cross-referenced this on Mar 16, 2018 from issue Follow standard Unix daemon practices by jgarzik
Empact cross-referenced this on Apr 11, 2018 from issue [Trivial] Ignore macOS daemon() depracation warning by jonasschnelli
andvgal referenced this in commit 88c4a0abd3 on Jan 6, 2019
bitcoin locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#4676 Follow standard Unix daemon practices #8278 Forking daemon #9005 OS-specific background service for bitcoind #9284 Suppress some annoying deprecation warnings (OSX)#10132 bitcoind ignores -printtoconsole flag #12941 [Trivial] Ignore macOS daemon() depracation warning