supernova deadlocks due to improper use of subprocess module

[coreywright]

supernova deadlocks because its usage of Popen, PIPE, wait(), and
stderr.read() doesn't heed the warnings in the Python documentation.

example

$ # watch supernova timeout with `nova --debug list`
$ (TIMEFORMAT='supernova took %0R seconds'; time timeout 60s supernova --debug test-env list --all-tenants 1 >/dev/null 2>&1; test $? -eq 124 && echo supernova timed out)
supernova took 60 seconds
supernova timed out
$ # patch supernova to heed warnings in subprocess module doc
$ patch venv/lib/python2.7/site-packages/supernova/supernova.py avoid_deadlock_with_subprocess.patch 
patching file venv/lib/python2.7/site-packages/supernova/supernova.py
$ # watch same supernova command succeed now
$ (TIMEFORMAT='supernova took %0R seconds'; time timeout 10s supernova --debug test-env list --all-tenants 1 >/dev/null 2>&1; test $? -eq 124 && echo supernova timed out)
supernova took 4 seconds
$ # revert patch to verify behavior also reverts to timing out
$ patch -R venv/lib/python2.7/site-packages/supernova/supernova.py avoid_deadlock_with_subprocess.patch 
patching file venv/lib/python2.7/site-packages/supernova/supernova.py
$ # watch supernova timeout, again
$ (TIMEFORMAT='supernova took %0R seconds'; time timeout 60s supernova --debug test-env list --all-tenants 1 >/dev/null 2>&1; test $? -eq 124 && echo supernova timed out)
supernova took 60 seconds
supernova timed out

avoid_deadlock_with_subprocess.patch

--- a/supernova.py	2017-05-18 13:51:58.555319000 -0500
+++ b/supernova.py	2017-08-21 19:03:38.016610165 -0500
@@ -40,8 +40,8 @@ def execute_executable(nova_args, env_va
                                stdout=sys.stdout,
                                stderr=subprocess.PIPE,
                                env=env_vars)
-    process.wait()
-    return process
+    stderr = process.communicate()[1]
+    return process, stderr
 
 
 def check_for_debug(supernova_args, nova_args):
@@ -88,17 +88,15 @@ def check_for_bypass_url(raw_creds, nova
     return nova_args
 
 
-def handle_stderr(stderr_pipe):
+def handle_stderr(stderr):
     """
     Takes stderr from the command's output and displays it AFTER the stdout
     is printed by run_command().
     """
-    stderr_output = stderr_pipe.read()
-
-    if len(stderr_output) > 0:
+    if len(stderr) > 0:
         click.secho("\n__ Error Output {0}".format('_'*62), fg='white',
                     bold=True)
-        click.echo(stderr_output)
+        click.echo(stderr)
 
     return True
 
@@ -141,10 +139,10 @@ def run_command(nova_creds, nova_args, s
     # In other news, I hate how python 2.6 does unicode.
     nova_args.insert(0, supernova_args['executable'])
     nova_args = [nova_arg.strip() for nova_arg in nova_args]
-    process = execute_executable(nova_args, env_vars)
+    process, stderr = execute_executable(nova_args, env_vars)
 
     # If the user asked us to be quiet, then let's not print stderr
     if not supernova_args.get('quiet'):
-        handle_stderr(process.stderr)
+        handle_stderr(stderr)
 
     return process.returncode

references

https://docs.python.org/2/library/subprocess.html#subprocess.Popen.wait

Warning: This will deadlock when using stdout=PIPE and/or stderr=PIPE and
the child process generates enough output to a pipe such that it blocks
waiting for the OS pipe buffer to accept more data. Use communicate() to
avoid that.

https://docs.python.org/2/library/subprocess.html#subprocess.Popen.stderr

Warning: Use communicate() rather than .stdin.write, .stdout.read or
.stderr.read to avoid deadlocks due to any of the other OS pipe buffers
filling up and blocking the child process.