From 4fd9509b77ba09bfe29e8bdda119fac6b879c436 Mon Sep 17 00:00:00 2001 From: Aaron Rosen Date: Mon, 22 Sep 2014 16:10:46 -0700 Subject: [PATCH] Replace wait with communicate to avoid potential deadlock Wait can 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. Change-Id: I452de95834a18a970ae78ab6c43d794151ce3300 closes-bug: 1372680 --- tempest/scenario/manager.py | 6 +++--- tempest/stress/actions/ssh_floating.py | 2 +- tempest/tests/test_wrappers.py | 7 ++----- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/tempest/scenario/manager.py b/tempest/scenario/manager.py index aa2d686062..3f2c13b6c0 100644 --- a/tempest/scenario/manager.py +++ b/tempest/scenario/manager.py @@ -624,7 +624,7 @@ class NeutronScenarioTest(ScenarioTest): proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - proc.wait() + proc.communicate() return (proc.returncode == 0) == should_succeed return tempest.test.call_until_true( @@ -1832,7 +1832,7 @@ class NetworkScenarioTest(OfficialClientTest): proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - proc.wait() + proc.communicate() return (proc.returncode == 0) == should_succeed return tempest.test.call_until_true( @@ -2267,7 +2267,7 @@ class OrchestrationScenarioTest(ScenarioTest): proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - proc.wait() + proc.communicate() return (proc.returncode == 0) == should_succeed return tempest.test.call_until_true( diff --git a/tempest/stress/actions/ssh_floating.py b/tempest/stress/actions/ssh_floating.py index 478cd07a72..d78112c1f0 100644 --- a/tempest/stress/actions/ssh_floating.py +++ b/tempest/stress/actions/ssh_floating.py @@ -30,7 +30,7 @@ class FloatingStress(stressaction.StressAction): proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - proc.wait() + proc.communicate() success = proc.returncode == 0 return success diff --git a/tempest/tests/test_wrappers.py b/tempest/tests/test_wrappers.py index 3f4ac7d3b4..0fd41f9a15 100644 --- a/tempest/tests/test_wrappers.py +++ b/tempest/tests/test_wrappers.py @@ -62,14 +62,11 @@ class TestWrappers(base.TestCase): p = subprocess.Popen( "bash %s" % cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - # wait in the general case is dangerous, however the amount of - # data coming back on those pipes is small enough it shouldn't be - # a problem. - p.wait() + out, err = p.communicate() self.assertEqual( p.returncode, expected, - "Stdout: %s; Stderr: %s" % (p.stdout.read(), p.stderr.read())) + "Stdout: %s; Stderr: %s" % (out, err)) def test_pretty_tox(self): # Git init is required for the pbr testr command. pbr requires a git