=== modified file 'DBUS-API' --- DBUS-API 2014-08-10 14:13:02 +0000 +++ DBUS-API 2015-05-23 20:18:34 +0000 @@ -134,10 +134,10 @@ k) A raw byte array, not hexadecimal digits. ** Signals -*** CheckerCompleted(n: Exitcode, x: Waitstatus, s: Command) +*** CheckerCompleted(n: Exitcode, x: Signal, s: Command) A checker (Command) has completed. Exitcode is either the exit - code or -1 for abnormal exit. In any case, the full Waitstatus - (as from wait(2)) is also available. + code or -1 for abnormal exit, in which case, the signal number + is available. *** CheckerStarted(s: Command) A checker command (Command) has just been started. @@ -155,8 +155,8 @@ * Copyright - Copyright © 2010-2012 Teddy Hogeborn - Copyright © 2010-2012 Björn Påhlsson + Copyright © 2010-2015 Teddy Hogeborn + Copyright © 2010-2015 Björn Påhlsson ** License: === modified file 'mandos' --- mandos 2015-05-31 19:05:32 +0000 +++ mandos 2015-05-31 20:12:27 +0000 @@ -424,6 +424,15 @@ .format(self.name))) return ret +def call_pipe(connection, # : multiprocessing.Connection + func, *args, **kwargs): + """This function is meant to be called by multiprocessing.Process + + This function runs func(*args, **kwargs), and writes the resulting + return value on the provided multiprocessing.Connection. + """ + connection.send(func(*args, **kwargs)) + connection.close() class Client(object): """A representation of a client host served by this server. @@ -456,6 +465,8 @@ last_checker_status: integer between 0 and 255 reflecting exit status of last checker. -1 reflects crashed checker, -2 means no checker completed yet. + last_checker_signal: The signal which killed the last checker, if + last_checker_status is -1 last_enabled: datetime.datetime(); (UTC) or None name: string; from the config file, used in log messages and D-Bus identifiers @@ -635,12 +646,18 @@ # Also start a new checker *right now*. self.start_checker() - def checker_callback(self, pid, condition, command): + def checker_callback(self, source, condition, connection, + command): """The checker has completed, so take appropriate actions.""" self.checker_callback_tag = None self.checker = None - if os.WIFEXITED(condition): - self.last_checker_status = os.WEXITSTATUS(condition) + # Read return code from connection (see call_pipe) + returncode = connection.recv() + connection.close() + + if returncode >= 0: + self.last_checker_status = returncode + self.last_checker_signal = None if self.last_checker_status == 0: logger.info("Checker for %(name)s succeeded", vars(self)) @@ -649,13 +666,16 @@ logger.info("Checker for %(name)s failed", vars(self)) else: self.last_checker_status = -1 + self.last_checker_signal = -returncode logger.warning("Checker for %(name)s crashed?", vars(self)) + return False def checked_ok(self): """Assert that the client has been seen, alive and well.""" self.last_checked_ok = datetime.datetime.utcnow() self.last_checker_status = 0 + self.last_checker_signal = None self.bump_timeout() def bump_timeout(self, timeout=None): @@ -687,20 +707,10 @@ # than 'timeout' for the client to be disabled, which is as it # should be. - # If a checker exists, make sure it is not a zombie - try: - pid, status = os.waitpid(self.checker.pid, os.WNOHANG) - except AttributeError: - pass - except OSError as error: - if error.errno != errno.ECHILD: - raise - else: - if pid: - logger.warning("Checker was a zombie") - gobject.source_remove(self.checker_callback_tag) - self.checker_callback(pid, status, - self.current_checker_command) + if self.checker is not None and not self.checker.is_alive(): + logger.warning("Checker was not alive; joining") + self.checker.join() + self.checker = None # Start a new checker if needed if self.checker is None: # Escape attributes for the shell @@ -715,46 +725,31 @@ exc_info=error) return True # Try again later self.current_checker_command = command - try: - logger.info("Starting checker %r for %s", command, - self.name) - # We don't need to redirect stdout and stderr, since - # in normal mode, that is already done by daemon(), - # and in debug mode we don't want to. (Stdin is - # always replaced by /dev/null.) - # The exception is when not debugging but nevertheless - # running in the foreground; use the previously - # created wnull. - popen_args = {} - if (not self.server_settings["debug"] - and self.server_settings["foreground"]): - popen_args.update({"stdout": wnull, - "stderr": wnull }) - self.checker = subprocess.Popen(command, - close_fds=True, - shell=True, - cwd="/", - **popen_args) - except OSError as error: - logger.error("Failed to start subprocess", - exc_info=error) - return True - self.checker_callback_tag = gobject.child_watch_add( - self.checker.pid, self.checker_callback, data=command) - # The checker may have completed before the gobject - # watch was added. Check for this. - try: - pid, status = os.waitpid(self.checker.pid, os.WNOHANG) - except OSError as error: - if error.errno == errno.ECHILD: - # This should never happen - logger.error("Child process vanished", - exc_info=error) - return True - raise - if pid: - gobject.source_remove(self.checker_callback_tag) - self.checker_callback(pid, status, command) + logger.info("Starting checker %r for %s", command, + self.name) + # We don't need to redirect stdout and stderr, since + # in normal mode, that is already done by daemon(), + # and in debug mode we don't want to. (Stdin is + # always replaced by /dev/null.) + # The exception is when not debugging but nevertheless + # running in the foreground; use the previously + # created wnull. + popen_args = { "close_fds": True, + "shell": True, + "cwd": "/" } + if (not self.server_settings["debug"] + and self.server_settings["foreground"]): + popen_args.update({"stdout": wnull, + "stderr": wnull }) + pipe = multiprocessing.Pipe(duplex = False) + self.checker = multiprocessing.Process( + target = call_pipe, + args = (pipe[1], subprocess.call, command), + kwargs = popen_args) + self.checker.start() + self.checker_callback_tag = gobject.io_add_watch( + pipe[0].fileno(), gobject.IO_IN, + self.checker_callback, pipe[0], command) # Re-run this periodically if run by gobject.timeout_add return True @@ -766,14 +761,7 @@ if getattr(self, "checker", None) is None: return logger.debug("Stopping checker for %(name)s", vars(self)) - try: - self.checker.terminate() - #time.sleep(0.5) - #if self.checker.poll() is None: - # self.checker.kill() - except OSError as error: - if error.errno != errno.ESRCH: # No such process - raise + self.checker.terminate() self.checker = None @@ -1111,7 +1099,7 @@ interface_names.add(alt_interface) # Is this a D-Bus signal? if getattr(attribute, "_dbus_is_signal", False): - if sys.version == 2: + if sys.version_info.major == 2: # Extract the original non-method undecorated # function by black magic nonmethod_func = (dict( @@ -1123,7 +1111,7 @@ # Create a new, but exactly alike, function # object, and decorate it to be a new D-Bus signal # with the alternate D-Bus interface name - if sys.version == 2: + if sys.version_info.major == 2: new_function = types.FunctionType( nonmethod_func.func_code, nonmethod_func.func_globals, @@ -1368,24 +1356,24 @@ DBusObjectWithProperties.__del__(self, *args, **kwargs) Client.__del__(self, *args, **kwargs) - def checker_callback(self, pid, condition, command, - *args, **kwargs): - self.checker_callback_tag = None - self.checker = None - if os.WIFEXITED(condition): - exitstatus = os.WEXITSTATUS(condition) + def checker_callback(self, source, condition, + connection, command, *args, **kwargs): + ret = Client.checker_callback(self, source, condition, + connection, command, *args, + **kwargs) + exitstatus = self.last_checker_status + if exitstatus >= 0: # Emit D-Bus signal self.CheckerCompleted(dbus.Int16(exitstatus), - dbus.Int64(condition), + dbus.Int64(0), dbus.String(command)) else: # Emit D-Bus signal self.CheckerCompleted(dbus.Int16(-1), - dbus.Int64(condition), + dbus.Int64( + self.last_checker_signal), dbus.String(command)) - - return Client.checker_callback(self, pid, condition, command, - *args, **kwargs) + return ret def start_checker(self, *args, **kwargs): old_checker_pid = getattr(self.checker, "pid", None) @@ -2660,7 +2648,7 @@ pass # Clients who has passed its expire date can still be - # enabled if its last checker was successful. Clients + # enabled if its last checker was successful. A Client # whose checker succeeded before we stored its state is # assumed to have successfully run all checkers during # downtime. === modified file 'mandos-monitor' --- mandos-monitor 2014-10-05 20:08:58 +0000 +++ mandos-monitor 2015-05-23 20:18:34 +0000 @@ -3,8 +3,8 @@ # # Mandos Monitor - Control and monitor the Mandos server # -# Copyright © 2009-2014 Teddy Hogeborn -# Copyright © 2009-2014 Björn Påhlsson +# Copyright © 2009-2015 Teddy Hogeborn +# Copyright © 2009-2015 Björn Påhlsson # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -173,7 +173,7 @@ gobject.source_remove(self._update_timer_callback_tag) self._update_timer_callback_tag = None - def checker_completed(self, exitstatus, condition, command): + def checker_completed(self, exitstatus, signal, command): if exitstatus == 0: self.logger('Checker for client {} (command "{}")' ' succeeded'.format(self.properties["Name"], @@ -181,20 +181,16 @@ self.update() return # Checker failed - if os.WIFEXITED(condition): + if exitstatus >= 0: self.logger('Checker for client {} (command "{}") failed' ' with exit code {}' .format(self.properties["Name"], command, - os.WEXITSTATUS(condition))) - elif os.WIFSIGNALED(condition): + exitstatus)) + elif signal != 0: self.logger('Checker for client {} (command "{}") was' ' killed by signal {}' .format(self.properties["Name"], command, - os.WTERMSIG(condition))) - elif os.WCOREDUMP(condition): - self.logger('Checker for client {} (command "{}") dumped' - ' core'.format(self.properties["Name"], - command)) + signal)) else: self.logger('Checker for client {} completed' ' mysteriously'