Avoid killing all available processes if an X error arrives early on

The timeline of events in dbus-launch's main process goes something like this:

* do initial X calls
[1]
* do some other stuff
* fork
    (child process starts doing some other stuff)
* return "intermediate parent" pid from fork()
* obtain bus daemon pid from bus_pid_to_launcher_pipe
[2]
* do things that might include X11 calls or killing the dbus-daemon

Meanwhile, the "babysitter" child goes like this:

* return 0 from fork()
[3]
* obtain bus daemon pid from parent process via bus_pid_to_babysitter_pipe
[4]
* do things that might include X11 calls or killing the bus daemon

Before [1] or [3], the right thing to do about an X error is to just
exit. The current implementation called kill(-1) first, which is
undesirable: it kills unrelated processes. With this change, we
just exit.

After [2] or [4], the right thing to do is to kill the dbus-daemon,
and that's what the existing code did.

Between [1] and [2], or between [3] and [4], there is no correct thing
that we can do immediately: we would have to wait for the end of the
"critical section", *then* kill the dbus-daemon. This has not yet been
implemented, so this patch relies for its correctness on the fact that
there are no libX11 calls between those points, so we cannot receive
an X error between them.

dbus-launch deserves more comments, or a reimplementation that is easier to
understand, but this change is certainly better than nothing.

[Commit message added, summarizing reviewers' comments -smcv]
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=74698
Reviewed-by: Simon McVittie
Reviewed-by: Thiago Macieira

diff --git a/tools/dbus-launch.c b/tools/dbus-launch.c
index 7ecee63e4d925189b093fc053970dffd1b01cce0..58a0322e5daf163fe4139bffd3f4ff42a94f54f4 100644 (file)
--- a/tools/dbus-launch.c
+++ b/tools/dbus-launch.c
@@ -406,6 +406,9 @@ static pid_t bus_pid_to_kill = -1;
 static void
 kill_bus(void)
 {
+  if (bus_pid_to_kill <= 0)
+    return;
+
   verbose ("Killing message bus and exiting babysitter\n");
   kill (bus_pid_to_kill, SIGTERM);
   sleep (3);
@@ -1275,6 +1278,10 @@ main (int argc, char **argv)
 
       bus_pid = val;
 
+      /* Have to initialize bus_pid_to_kill ASAP, so that the
+         X error callback can kill it if an error happens. */
+      bus_pid_to_kill = bus_pid;
+
       close (bus_pid_to_launcher_pipe[READ_END]);
 
 #ifdef DBUS_ENABLE_X11_AUTOLAUNCH
@@ -1291,7 +1298,6 @@ main (int argc, char **argv)
                 {
                   char *address = NULL;
                   /* another window got added. Return its address */
-                  bus_pid_to_kill = bus_pid;
                   if (x11_get_address (&address, &bus_pid, &wid)
                        && address != NULL)
                     {