On Thu, 2014-06-26 at 09:24 -0400, David Shea wrote:
On 06/26/2014 05:48 AM, Vratislav Podzimek wrote:
> 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))
> Can we define a special exception class for this exception and use it
> here?
>
Sure, ExitError or something like that. I can make it derive from
RuntimeError, and the try-except block around startX11 will already
cover it.
Deriving from RuntimeError sounds best to me. I was thinking about
suggesting RuntimeError directly as it is.
--
Vratislav Podzimek
Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic