Skip to content

Commit

Permalink
Avoid deadlock with subprocess
Browse files Browse the repository at this point in the history
Heed 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 3af09c4
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 3af09c4

Please sign in to comment.