From 1ab6e7ab38c71ea368dceed753d376580efee805 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Wed, 5 Jun 2013 19:19:08 +0100 Subject: [PATCH] dbus-launch: run dbus-daemon with --nofork Instead of running dbus-daemon --fork, do the 75% of it that we actually want. Under systemd, we want to leave stderr open, so that any stderr from either dbus-daemon or the services it activated will be logged to the journal; under non-systemd, stderr will either be somewhere useful, or already /dev/null. --- tools/dbus-launch.c | 107 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 68 insertions(+), 39 deletions(-) diff --git a/tools/dbus-launch.c b/tools/dbus-launch.c index fc55f9b..f65c5f9 100644 --- a/tools/dbus-launch.c +++ b/tools/dbus-launch.c @@ -50,8 +50,8 @@ extern Display *xdisplay; * shell [*] * \- main() --exec--> myapp[*] * \- "intermediate parent" - * \- bus-runner --exec--> dbus-daemon --fork - * \- babysitter[*] \- final dbus-daemon[*] + * \- bus-runner --exec--> dbus-daemon --nofork [*] + * \- babysitter[*] * * Processes marked [*] survive the initial flurry of activity. * @@ -72,12 +72,6 @@ extern Display *xdisplay; * reap its children. We *can* rely on main() to reap the intermediate parent, * because that happens before it execs myapp. * - * It's unclear why dbus-daemon needs to fork, but we explicitly tell it to - * for some reason, then wait for it. If we left it undefined, a forking - * dbus-daemon would get the parent process reparented to init and reaped - * when the intermediate parent terminated, and a non-forking dbus-daemon - * would get reparented to init and carry on there. - * * myapp is exec'd by the process that initially ran main() so that it's * the shell's child, so the shell knows how to do job control and stuff. * This is desirable for the "dbus-launch an application" use-case, less so @@ -595,12 +589,55 @@ kill_bus_when_session_ends (void) } static void +close_standard_fds (int close_stdin, + int close_stdout, + int close_stderr) +{ + int dev_null_fd; + + /* Close stdout/stderr so we don't block an "eval" or otherwise + * lock up. stdout is still chaining through to dbus-launch + * and in turn to the parent shell. + */ + dev_null_fd = open ("/dev/null", O_RDWR); + if (dev_null_fd >= 0) + { + if (close_stdin) + dup2 (dev_null_fd, 0); + + if (close_stdout) + dup2 (dev_null_fd, 1); + + if (close_stderr) + dup2 (dev_null_fd, 2); + + close (dev_null_fd); + } + else + { + fprintf (stderr, "Failed to open /dev/null: %s\n", + strerror (errno)); + /* continue, why not */ + } +} + +static void +cd_to_root_or_die (void) +{ + if (chdir ("/") < 0) + { + fprintf (stderr, "Could not change to root directory: %s\n", + strerror (errno)); + exit (1); + } +} + +static void babysit (int exit_with_session, pid_t child_pid, int read_bus_pid_fd) /* read pid from here */ { int ret; - int dev_null_fd; const char *s; verbose ("babysitting, exit_with_session = %d, child_pid = %ld, read_bus_pid_fd = %d\n", @@ -612,35 +649,17 @@ babysit (int exit_with_session, * to the tty and the X server in order to kill the message bus * when the session ends. */ - - if (chdir ("/") < 0) - { - fprintf (stderr, "Could not change to root directory: %s\n", - strerror (errno)); - exit (1); - } + cd_to_root_or_die (); /* Close stdout/stderr so we don't block an "eval" or otherwise * lock up. stdout is still chaining through to dbus-launch * and in turn to the parent shell. */ - dev_null_fd = open ("/dev/null", O_RDWR); - if (dev_null_fd >= 0) - { - if (!exit_with_session) - dup2 (dev_null_fd, 0); - dup2 (dev_null_fd, 1); - s = getenv ("DBUS_DEBUG_OUTPUT"); - if (s == NULL || *s == '\0') - dup2 (dev_null_fd, 2); - close (dev_null_fd); - } - else - { - fprintf (stderr, "Failed to open /dev/null: %s\n", - strerror (errno)); - /* continue, why not */ - } + s = getenv ("DBUS_DEBUG_OUTPUT"); + close_standard_fds ( + !exit_with_session, /* stdin */ + 1, /* stdout */ + (s == NULL || *s == '\0') /* stderr */); ret = fork (); @@ -1111,14 +1130,24 @@ main (int argc, char **argv) verbose ("=== Bus exec process created\n"); - /* Now we are the bus process (well, almost; - * dbus-daemon itself forks again) - */ + /* Now we are the bus process. */ close (bus_pid_to_launcher_pipe[READ_END]); close (bus_address_to_launcher_pipe[READ_END]); close (bus_pid_to_babysitter_pipe[READ_END]); close (bus_pid_to_babysitter_pipe[WRITE_END]); + /* Do about 75% of _dbus_become_daemon(), but don't fork(). */ + + cd_to_root_or_die (); + + /* stderr has already been closed, if appropriate. */ + close_standard_fds (1, 1, 0); + + /* Release our controlling terminal. + * Can't fail: we're a newly-created process, so we are not already + * a process group leader */ + setsid (); + sprintf (write_pid_fd_as_string, "%d", bus_pid_to_launcher_pipe[WRITE_END]); @@ -1133,7 +1162,7 @@ main (int argc, char **argv) { execl (TEST_BUS_BINARY, TEST_BUS_BINARY, - "--fork", + "--nofork", "--print-pid", write_pid_fd_as_string, "--print-address", write_address_fd_as_string, config_file ? "--config-file" : "--session", @@ -1148,7 +1177,7 @@ main (int argc, char **argv) execl (DBUS_DAEMONDIR"/dbus-daemon", DBUS_DAEMONDIR"/dbus-daemon", - "--fork", + "--nofork", "--print-pid", write_pid_fd_as_string, "--print-address", write_address_fd_as_string, config_file ? "--config-file" : "--session", @@ -1167,7 +1196,7 @@ main (int argc, char **argv) */ execlp ("dbus-daemon", "dbus-daemon", - "--fork", + "--nofork", "--print-pid", write_pid_fd_as_string, "--print-address", write_address_fd_as_string, config_file ? "--config-file" : "--session", -- 1.7.10.4