Fix test race conditions causing cluster_tests.py:management_test to hang.

Got rid of expect_fail, ignore exceptions in the caller.
Avoid concurrent calls to _cleanup, call only from from wait/poll.


git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@984017 13f79535-47bb-0310-9956-ffa450edef68

2 files changed, 18 insertions, 24 deletions

diff --git a/qpid/cpp/src/tests/cluster_tests.py b/qpid/cpp/src/tests/cluster_tests.py
index 00cd98a20a..b4fa510528 100755
--- a/qpid/cpp/src/tests/cluster_tests.py
+++ b/qpid/cpp/src/tests/cluster_tests.py
@@ -259,7 +259,6 @@ class LongTests(BrokerTest):
                 self.cmd = cmd          # Client command.
                 self.lock = Lock()
                 self.process = None     # Client process.
-                self._expect_fail = False
                 self.start()
 
             def run(self):
@@ -284,19 +283,13 @@ class LongTests(BrokerTest):
                         try:
                             # Quit and ignore errors if stopped or expecting failure.
                             if self.stopped: break
-                            if not self._expect_fail and exit != 0:
+                            if exit != 0:
                                 self.process.unexpected(
                                     "client of %s exit code %s"%(self.broker.name, exit))
                         finally: self.lock.release()
                 except Exception, e:
                     self.error = RethrownException("Error in ClientLoop.run")
 
-            def expect_fail(self):
-                """Ignore exit status of the currently running client."""
-                self.lock.acquire()
-                self._expect_fail = True
-                self.lock.release()
-                
             def stop(self):
                 """Stop the running client and wait for it to exit"""
                 self.lock.acquire()
@@ -342,12 +335,13 @@ class LongTests(BrokerTest):
             time.sleep(max(5,self.duration()/4))
             for b in cluster[alive:]: b.ready() # Check if a broker crashed.
             # Kill the first broker, expect the clients to fail. 
-            for c in clients[alive] + mclients: c.expect_fail()
             b = cluster[alive]
             b.expect = EXPECT_EXIT_FAIL
             b.kill()
-            # Stop the brokers clients and all the mclients.
-            for c in clients[alive] + mclients: c.stop()
+            # Stop the brokers clients and all the mclients. 
+            for c in clients[alive] + mclients:
+                try: c.stop()
+                except: pass            # Ignore expected errors due to broker shutdown.
             clients[alive] = []
             mclients = []
             # Start another broker and clients
diff --git a/qpid/python/qpid/brokertest.py b/qpid/python/qpid/brokertest.py
index 19acfd76ca..523298a492 100644
--- a/qpid/python/qpid/brokertest.py
+++ b/qpid/python/qpid/brokertest.py
@@ -186,7 +186,6 @@ class Popen(popen2.Popen3):
         finally: self._clean_lock.release()
 
     def unexpected(self,msg):
-        self._cleanup()
         err = error_line(self.outfile("err")) or error_line(self.outfile("out"))
         raise BadProcessStatus("%s %s%s" % (self.pname, msg, err))
     
@@ -210,7 +209,7 @@ class Popen(popen2.Popen3):
                 elif self.expect == EXPECT_EXIT_FAIL and self.returncode == 0:
                     self.unexpected("expected error")
         finally:
-            self._cleanup()
+            self.wait()                 # Clean up the process.
                
     def communicate(self, input=None):
         if input:
@@ -227,26 +226,27 @@ class Popen(popen2.Popen3):
         if not self.is_running(): self.unexpected("Exit code %d" % self.returncode)
 
     def poll(self):
-        if self.returncode is not None: return self.returncode
-        self.returncode = popen2.Popen3.poll(self)
-        if (self.returncode == -1): self.returncode = None
-        if self.returncode is not None: self._cleanup()
+        if self.returncode is None:
+            ret = popen2.Popen3.poll(self)
+            if (ret != -1):
+                self.returncode = ret
+                self._cleanup()
         return self.returncode
 
     def wait(self):
-        if self.returncode is not None: return self.returncode
-        self.drain()
-        try: self.returncode = popen2.Popen3.wait(self)
-        except OSError,e: raise OSError("Wait failed %s: %s"%(self.pname, e))
-        self._cleanup()
+        if self.returncode is None:
+            self.drain()
+            try: self.returncode = popen2.Popen3.wait(self)
+            except OSError,e: raise OSError("Wait failed %s: %s"%(self.pname, e))
+            self._cleanup()
         return self.returncode
 
     def send_signal(self, sig):
         try: os.kill(self.pid,sig)
         except OSError,e: raise OSError("Kill failed %s: %s"%(self.pname, e))
 
-    def terminate(self): self.send_signal(signal.SIGTERM); self._cleanup()
-    def kill(self): self.send_signal(signal.SIGKILL); self._cleanup()
+    def terminate(self): self.send_signal(signal.SIGTERM) 
+    def kill(self): self.send_signal(signal.SIGKILL)
 
     def cmd_str(self): return " ".join([str(s) for s in self.cmd])