From 3b39bc56d38511e10871447392b709b3b8e65637 Mon Sep 17 00:00:00 2001 From: Ruben Rodriguez Buchillon Date: Tue, 21 Aug 2018 08:16:02 +0800 Subject: ec3po: quit console & interpreter loop when parent process changes. Right now, when the parent process dies ungracefully - say kill -9 - then the interpreter, and console processes remain active. This leads to bugs in the servod implementation from holding on to sockets, to reinitialization issues of a new instance on the same servod device. This change quits the loops inside console & interpreter as soon as the parent pid changes (i.e. the parent dies). BRANCH=None BUG=chromium:841121 TEST=sudo servod -b soraka ps aux | grep servod >xxxxx servod >xxxxy servod >xxxyx servod >xxxaa grep servod sudo kill -9 xxxxx ps aux | grep servod >xxxab grep servod Before this, just kill -9 on the main thread did not take the children with it. Change-Id: I547bd92bf8732bff8aef2b72840417c809ba27d6 Signed-off-by: Ruben Rodriguez Buchillon Reviewed-on: https://chromium-review.googlesource.com/1186299 Reviewed-by: Aseda Aboagye Reviewed-by: Nick Sanders --- util/ec3po/console.py | 7 ++++--- util/ec3po/interpreter.py | 9 ++++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/util/ec3po/console.py b/util/ec3po/console.py index 608e1b7cba..0b2b586252 100755 --- a/util/ec3po/console.py +++ b/util/ec3po/console.py @@ -817,13 +817,14 @@ def IsPrintable(byte): """ return byte >= ord(' ') and byte <= ord('~') -def StartLoop(console, command_active): +def StartLoop(console, command_active, ppid = os.getppid()): """Starts the infinite loop of console processing. Args: console: A Console object that has been properly initialzed. command_active: multiprocessing.Value indicating if servod owns the console, or user owns the console. This prevents input collisions. + ppid: original parent pid to stop loop when parent dies. """ console.logger.debug('Console is being served on %s.', console.user_pty) console.logger.debug('Console master is on %s.', console.master_pty) @@ -836,7 +837,7 @@ def StartLoop(console, command_active): ep = select.epoll() ep.register(console.master_pty, select.EPOLLHUP) - while True: + while os.getppid() == ppid: # Check to see if pts is connected to anything events = ep.poll(0) master_connected = not events @@ -980,7 +981,7 @@ def main(argv): # Spawn an interpreter process. itpr_process = multiprocessing.Process(target=interpreter.StartLoop, - args=(itpr,)) + args=(itpr,os.getpid())) # Make sure to kill the interpreter when we terminate. itpr_process.daemon = True # Start the interpreter. diff --git a/util/ec3po/interpreter.py b/util/ec3po/interpreter.py index 9a0880f49c..6de8ff7a43 100644 --- a/util/ec3po/interpreter.py +++ b/util/ec3po/interpreter.py @@ -374,7 +374,7 @@ def Crc8(data): return crc >> 8 -def StartLoop(interp): +def StartLoop(interp, ppid = os.getppid()): """Starts an infinite loop of servicing the user and the EC. StartLoop checks to see if there are any commands to process, processing them @@ -389,10 +389,13 @@ def StartLoop(interp): Args: interp: An Interpreter object that has been properly initialised. + ppid: original parent pid to stop loop when parent dies. """ try: - while True: - readable, writeable, _ = select.select(interp.inputs, interp.outputs, []) + while os.getppid() == ppid: + # Timeout every 100ms to catch if the parent has died. + readable, writeable, _ = select.select(interp.inputs, interp.outputs, [], + 0.1) for obj in readable: # Handle any debug prints from the EC. -- cgit v1.2.1