Skip to content

Commit

Permalink
Avoid deadlock with subprocess
Browse files Browse the repository at this point in the history
Heed Python's subprocess module documentation warnings about deadlock
when using PIPE for a Popen object's stderr with Popen.wait() and
Popen.stderr.read() and use Popen.communicate() instead.

Fixes major#126.
  • Loading branch information
coreywright committed Aug 22, 2017
1 parent 4a217ae commit 67146a4
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 14 deletions.
16 changes: 7 additions & 9 deletions supernova/supernova.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ def execute_executable(nova_args, env_vars):
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):
Expand Down Expand Up @@ -88,17 +88,15 @@ def check_for_bypass_url(raw_creds, nova_args):
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

Expand Down Expand Up @@ -141,10 +139,10 @@ def run_command(nova_creds, nova_args, supernova_args):
# 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
9 changes: 4 additions & 5 deletions tests/test_supernova.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,9 @@ def test_debug_check_with_heat(self):
expected_nova_args = ['-d ']
assert result == expected_nova_args

def test_handle_stderr(self, tmpdir, capsys):
p = tmpdir.mkdir("sub").join("stderr.txt")
p.write("This would be in the stderr pipe")
result = supernova.handle_stderr(p)
def test_handle_stderr(self, capsys):
s = "This would be in the stderr pipe"
result = supernova.handle_stderr(s)
out, err = capsys.readouterr()
assert "stderr pipe" in out
assert s in out
assert result

0 comments on commit 67146a4

Please sign in to comment.