Attachment 80365 Details for Bug 64843 – dbus-launch: run dbus-daemon with --nofork

[patch] dbus-launch: run dbus-daemon with --nofork

0001-dbus-launch-run-dbus-daemon-with-nofork.patch (text/plain), 6.36 KB, created by Simon McVittie on 2013-06-05 18:31:54 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2013-06-05 18:31:54 UTC

Size: 6.36 KB

patch

obsolete

>From 1ab6e7ab38c71ea368dceed753d376580efee805 Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>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
>

Actions: View | Splinter Review

Attachments on bug 64843: 79633 | 80365

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.