From 2dd69bb5f2a2c8eae285626bd12dfa1aafb63b2b Mon Sep 17 00:00:00 2001
From: Daniel Malea <daniel.malea@intel.com>
Date: Fri, 15 Feb 2013 21:21:52 +0000
Subject: [PATCH] Fix misuse of python subprocess module (caused "leaking"
 processes and garbling the terminal) - fixed cleanup of Popen objects by
 pushing spawn logic into test Base and out of test cases - connect subprocess
 stdin to PIPE (rather than the parent's STDIN) to fix silent terminal issue

Tested on Linux and Mac OS X

llvm-svn: 175301
---
 .../test/functionalities/register/TestRegisters.py | 16 +++++--------
 lldb/test/lldbtest.py                              | 28 ++++++++++++++++++++++
 lldb/test/python_api/hello_world/TestHelloWorld.py | 16 +++++--------
 3 files changed, 40 insertions(+), 20 deletions(-)

diff --git a/lldb/test/functionalities/register/TestRegisters.py b/lldb/test/functionalities/register/TestRegisters.py
index 6ce30b0..15ed5b8 100644
--- a/lldb/test/functionalities/register/TestRegisters.py
+++ b/lldb/test/functionalities/register/TestRegisters.py
@@ -100,18 +100,14 @@ class RegisterCommandsTestCase(TestBase):
         """Test convenience registers after a 'process attach'."""
         exe = self.lldbHere
         
-        # Spawn a new process and don't display the stdout if not in TraceOn() mode.
-        import subprocess
-        popen = subprocess.Popen([exe, self.lldbOption],
-                                 stdout = open(os.devnull, 'w') if not self.TraceOn() else None)
-        if self.TraceOn():
-            print "pid of spawned process: %d" % popen.pid
+        # Spawn a new process
+        proc = self.spawnSubprocess(exe, [self.lldbOption])
+        self.addTearDownHook(self.cleanupSubprocesses)
 
-        self.runCmd("process attach -p %d" % popen.pid)
+        if self.TraceOn():
+            print "pid of spawned process: %d" % proc.pid
 
-        # Add a hook to kill the child process during teardown.
-        self.addTearDownHook(
-            lambda: popen.kill())
+        self.runCmd("process attach -p %d" % proc.pid)
 
         # Check that "register read eax" works.
         self.runCmd("register read eax")
diff --git a/lldb/test/lldbtest.py b/lldb/test/lldbtest.py
index fbfb269..af71627 100644
--- a/lldb/test/lldbtest.py
+++ b/lldb/test/lldbtest.py
@@ -614,6 +614,9 @@ class Base(unittest2.TestCase):
         self.dicts = []
         self.doTearDownCleanups = False
 
+        # List of spawned subproces.Popen objects
+        self.subprocesses = []
+
         # Create a string buffer to record the session info, to be dumped into a
         # test case specific file if test failure is encountered.
         self.session = StringIO.StringIO()
@@ -667,6 +670,31 @@ class Base(unittest2.TestCase):
                 child.sendline(hook)
                 child.expect_exact(child_prompt)
 
+    def cleanupSubprocesses(self):
+        # Ensure any subprocesses are cleaned up
+        for p in self.subprocesses:
+            if p.poll() == None:
+                p.terminate()
+            del p
+        del self.subprocesses[:]
+
+    def spawnSubprocess(self, executable, args=[]):
+        """ Creates a subprocess.Popen object with the specified executable and arguments,
+            saves it in self.subprocesses, and returns the object.
+            NOTE: if using this function, ensure you also call:
+
+              self.addTearDownHook(self.cleanupSubprocesses)
+
+            otherwise the test suite will leak processes.
+        """
+
+        # Don't display the stdout if not in TraceOn() mode.
+        proc = Popen([executable] + args,
+                     stdout = open(os.devnull) if not self.TraceOn() else None,
+                     stdin = PIPE)
+        self.subprocesses.append(proc)
+        return proc
+
     def HideStdout(self):
         """Hide output to stdout from the user.
 
diff --git a/lldb/test/python_api/hello_world/TestHelloWorld.py b/lldb/test/python_api/hello_world/TestHelloWorld.py
index acb583b..2b895b9 100644
--- a/lldb/test/python_api/hello_world/TestHelloWorld.py
+++ b/lldb/test/python_api/hello_world/TestHelloWorld.py
@@ -135,11 +135,9 @@ class HelloWorldTestCase(TestBase):
 
         target = self.dbg.CreateTarget(self.exe)
 
-        # Spawn a new process and don't display the stdout if not in TraceOn() mode.
-        import subprocess
-        popen = subprocess.Popen([self.exe, "abc", "xyz"],
-                                 stdout = open(os.devnull, 'w') if not self.TraceOn() else None)
-        #print "pid of spawned process: %d" % popen.pid
+        # Spawn a new process
+        popen = self.spawnSubprocess(self.exe, ["abc", "xyz"])
+        self.addTearDownHook(self.cleanupSubprocesses)
 
         listener = lldb.SBListener("my.attach.listener")
         error = lldb.SBError()
@@ -159,11 +157,9 @@ class HelloWorldTestCase(TestBase):
 
         target = self.dbg.CreateTarget(self.exe)
 
-        # Spawn a new process and don't display the stdout if not in TraceOn() mode.
-        import subprocess
-        popen = subprocess.Popen([self.exe, "abc", "xyz"],
-                                 stdout = open(os.devnull, 'w') if not self.TraceOn() else None)
-        #print "pid of spawned process: %d" % popen.pid
+        # Spawn a new process
+        popen = self.spawnSubprocess(self.exe, ["abc", "xyz"])
+        self.addTearDownHook(self.cleanupSubprocesses)
 
         listener = lldb.SBListener("my.attach.listener")
         error = lldb.SBError()
-- 
2.7.4