[PATCH 1/2] Remove surprises from X startup.
Vratislav Podzimek
vpodzime at redhat.com
Thu Jun 26 05:50:44 UTC 2014
On Wed, 2014-06-25 at 15:07 -0400, David Shea wrote:
> (this version doesn't try to mutate the dict it's iterating)
>
> As was noted in the comment, assuming that any SIGCHLD means that X
> exited makes the startup process fragile, so stop doing that. Install a
> global SIGCHLD handler to track exits from Xorg and metacity. metacity
> failures will now throw an exception instead of being silently,
> confusingly ignored. Add a fork wrapper to remove the race between
> process startup and process monitoring.
>
> Removed a comment about the importance of the window manager being the
> first X connection. This is no longer true, which is fortunate as the
> period between metacity starting and metacity connecting to the X server
> presents an intractible race.
> ---
> anaconda | 212 +++++++++++++++++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 165 insertions(+), 47 deletions(-)
>
> diff --git a/anaconda b/anaconda
> index cbbedd6..9bb270b 100755
> --- a/anaconda
> +++ b/anaconda
> @@ -44,9 +44,77 @@ if ("debug=1" in proc_cmdline) or ("debug" in proc_cmdline):
> cov.start()
>
>
> -import atexit, sys, os, time, subprocess
> -# keep up with process ID of the window manager if we start it
> -wm_pid = None
> +import atexit, sys, os, time, subprocess, signal, errno
> +
> +# Install a global SIGCHLD handler to keep track of things that should be
> +# running for as long as anaconda does. The dictionary is of the form
> +# {pid: name, ...}. The handler will raise OSError, so if not caught
> +# a SIGCHLD from a watched process will halt anaconda.
> +forever_pids = {}
> +
> +def sigchld_handler(num, frame):
> + # Check whether anything in the list of processes being watched has
> + # exited. We don't want to call waitpid(-1), since that would break
> + # anything else using wait/waitpid (like the subprocess module).
> + exited_pids = []
> + exn_message = []
> +
> + for child_pid in forever_pids:
> + try:
> + pid_result, status = os.waitpid(child_pid, os.WNOHANG)
> + except OSError as e:
> + if e.errno == errno.ECHILD:
> + continue
> +
> + if pid_result:
> + proc_name = forever_pids[child_pid]
> + exited_pids.append(child_pid)
> +
> + if os.WIFEXITED(status):
> + status_str = "with status %s" % os.WEXITSTATUS(status)
> + elif os.WIFSIGNALED(status):
> + status_str = "on signal %s" % os.WTERMSIG(status)
> + else:
> + status_str = "with unknown status code %s" % status
> +
> + exn_message.append("%s exited %s" % (proc_name, status_str))
> +
> + for child_pid in exited_pids:
> + del forever_pids[child_pid]
> +
> + if exn_message:
> + raise OSError(0, ", ".join(exn_message))
> +
> +signal.signal(signal.SIGCHLD, sigchld_handler)
> +
> +# Fork a new process and add it to forever_pids. The return values are the
> +# the same as os.fork, but neither the parent nor the child will return
> +# until the process is watched.
> +def start_watched_pid(proc_name):
> + readpipe, writepipe = os.pipe()
> + childpid = os.fork()
> + if not childpid:
Don't know about the others, but I find 'if childpid != 0' much better
readable and obvious than 'if not childpid'.
Other than that this looks good to me. Nice change!
--
Vratislav Podzimek
Anaconda Rider | RHCE | Red Hat, Inc. | Brno - Czech Republic
More information about the anaconda-patches
mailing list