Improve zombie process error message to be actionable.

client/run_isolated.py

 #!/usr/bin/env python
 # Copyright 2012 The LUCI Authors. All rights reserved.
 # Use of this source code is governed under the Apache License, Version 2.0
 # that can be found in the LICENSE file.
 
 """Runs a command with optional isolated input/output.
 
 Despite name "run_isolated", can run a generic non-isolated command specified as
 args.
 
@@ skipping 87 matching lines @@
 # It is recommended to start the script with a `root_dir` as short as
 # possible.
 # - ir stands for isolated_run
 # - io stands for isolated_out
 # - it stands for isolated_tmp
 ISOLATED_RUN_DIR = u'ir'
 ISOLATED_OUT_DIR = u'io'
 ISOLATED_TMP_DIR = u'it'
 
 
+OUTLIVING_ZOMBIE_MSG = """\
+*** Swarming tried multiple times to delete the %s directory and failed ***
+*** Hard failing the task ***
+
+Swarming detected that your testing script ran an executable, which may have
+started a child executable, and the main script returned early, leaving the
+children executables playing around unguided.
+
+You don't want to leave children processes outliving the task on the Swarming
+bot, do you? The Swarming bot doesn't.
+
+How to fix?
+- For any process that starts children processes, make sure all children
+  processes terminated properly before each parent process exits. This is
+  especially important in very deep process trees.
+  - This must be done properly both in normal successful task and in case of
+    task failure. Cleanup is very important.
+- The Swarming bot sends a SIGTERM in case of timeout.
+  - You have %s seconds to comply after the signal was sent to the process
+    before the process is forcibly killed.
+- To achieve not leaking children processes in case of signals on timeout, you
+  MUST handle signals in each executable / python script and propagate them to
+  children processes.
+  - When your test script (python or binary) receives a signal like SIGTERM or
+    CTRL_BREAK_EVENT on Windows), send it to all children processes and wait for
+    them to terminate before quitting.
+
+See
+https://github.com/luci/luci-py/blob/master/appengine/swarming/doc/Bot.md#graceful-termination-aka-the-sigterm-and-sigkill-dance
+for more information.
+
+*** May the SIGKILL force be with you ***
+"""
+
+
 def get_as_zip_package(executable=True):
   """Returns ZipPackage with this module and all its dependencies.
 
   If |executable| is True will store run_isolated.py as __main__.py so that
   zip package is directly executable be python.
   """
   # Building a zip package when running from another zip package is
   # unsupported and probably unneeded.
   assert not zip_package.is_zipped_module(sys.modules[__name__])
   assert THIS_FILE_PATH
@@ skipping 422 matching lines @@
         # process locks *.exe file). Examine out_dir only after that call
         # completes (since child processes may write to out_dir too and we need
         # to wait for them to finish).
         if fs.isdir(run_dir):
           try:
             success = file_path.rmtree(run_dir)
           except OSError as e:
             logging.error('Failure with %s', e)
             success = False
           if not success:
-            print >> sys.stderr, (
-                'Failed to delete the run directory, thus failing the task.\n'
-                'This may be due to a subprocess outliving the main task\n'
-                'process, holding on to resources. Please fix the task so\n'
-                'that it releases resources and cleans up subprocesses.')
+            sys.stderr.write(OUTLIVING_ZOMBIE_MSG % ('run', grace_period))
             if result['exit_code'] == 0:
               result['exit_code'] = 1
         if fs.isdir(tmp_dir):
           try:
             success = file_path.rmtree(tmp_dir)
           except OSError as e:
             logging.error('Failure with %s', e)
             success = False
           if not success:
-            print >> sys.stderr, (
-                'Failed to delete the temp directory, thus failing the task.\n'
-                'This may be due to a subprocess outliving the main task\n'
-                'process, holding on to resources. Please fix the task so\n'
-                'that it releases resources and cleans up subprocesses.')
+            sys.stderr.write(OUTLIVING_ZOMBIE_MSG % ('run', grace_period))

[Taylor_Brandstetter, 2017/06/08 17:58:46]

Should be "temp"?

[M-A Ruel, 2017/06/08 18:14:07]

Argh, I got caught copy-pasting. Fixing.

             if result['exit_code'] == 0:
               result['exit_code'] = 1
 
       # This deletes out_dir if leak_temp_dir is not set.
       if out_dir:
         isolated_stats = result['stats'].setdefault('isolated', {})
         result['outputs_ref'], success, isolated_stats['upload'] = (
             delete_and_upload(storage, out_dir, leak_temp_dir))
       if not success and result['exit_code'] == 0:
         result['exit_code'] = 1
@@ skipping 489 matching lines @@
     return 1
 
 
 if __name__ == '__main__':
   subprocess42.inhibit_os_error_reporting()
   # Ensure that we are always running with the correct encoding.
   fix_encoding.fix_encoding()
   file_path.enable_symlink()
 
   sys.exit(main(sys.argv[1:]))