From 3be6063729017060fbca41970fa1ab7768be37e4 Mon Sep 17 00:00:00 2001 From: =?utf8?q?=D0=A0=D0=BE=D0=BC=D0=B0=D0=BD=20=D0=94=D0=BE=D0=BD=D1=87?= =?utf8?q?=D0=B5=D0=BD=D0=BA=D0=BE?= Date: Wed, 30 Apr 2014 19:11:56 +0100 Subject: [PATCH] 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 --- tools/dbus-launch.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/dbus-launch.c b/tools/dbus-launch.c index 7ecee63e..58a0322e 100644 --- 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) { -- 2.34.1