[Lldb-commits] [lldb] r175301 - Fix misuse of python subprocess module (caused "leaking" processes and garbling the terminal)

[Daniel Malea]

Author: dmalea
Date: Fri Feb 15 15:21:52 2013
New Revision: 175301

URL: http://llvm.org/viewvc/llvm-project?rev=175301&view=rev
Log:
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


Modified:
    lldb/trunk/test/functionalities/register/TestRegisters.py
    lldb/trunk/test/lldbtest.py
    lldb/trunk/test/python_api/hello_world/TestHelloWorld.py

Modified: lldb/trunk/test/functionalities/register/TestRegisters.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/register/TestRegisters.py?rev=175301&r1=175300&r2=175301&view=diff
==============================================================================
--- lldb/trunk/test/functionalities/register/TestRegisters.py (original)
+++ lldb/trunk/test/functionalities/register/TestRegisters.py Fri Feb 15 15:21:52 2013
@@ -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")

Modified: lldb/trunk/test/lldbtest.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/lldbtest.py?rev=175301&r1=175300&r2=175301&view=diff
==============================================================================
--- lldb/trunk/test/lldbtest.py (original)
+++ lldb/trunk/test/lldbtest.py Fri Feb 15 15:21:52 2013
@@ -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.
 

Modified: lldb/trunk/test/python_api/hello_world/TestHelloWorld.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/python_api/hello_world/TestHelloWorld.py?rev=175301&r1=175300&r2=175301&view=diff
==============================================================================
--- lldb/trunk/test/python_api/hello_world/TestHelloWorld.py (original)
+++ lldb/trunk/test/python_api/hello_world/TestHelloWorld.py Fri Feb 15 15:21:52 2013
@@ -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()