Do not use a pseudo-terminal in command_executer.

Using a psuedo-terminal causes problems when running jobs through cron. Gentoo's
sandbox aborts builds due to a write violation to /dev/ptmx. Longer term
Gentoo's sandbox may need to add /dev/ptmx to the whitelist.

BUG=none
TEST=python no_pseudo_terminal_test.py passes.

Change-Id: I2ac00718a4694f097fcec8bcf2cc95d48bdd9e51
Reviewed-on: https://gerrit-int.chromium.org/37323
Reviewed-by: Bhaskar Janakiraman <bjanakiraman@google.com>
Commit-Queue: asharif <asharif@google.com>
Tested-by: asharif <asharif@google.com>

2 files changed, 50 insertions, 5 deletions

diff --git a/utils/command_executer.py b/utils/command_executer.py
index 0aedc47c..a4dca640 100644
--- a/utils/command_executer.py
+++ b/utils/command_executer.py
@@ -5,7 +5,6 @@
 
 import getpass
 import os
-import pty
 import re
 import select
 import subprocess
@@ -62,10 +61,9 @@ class CommandExecuter:
         user = username + "@"
       cmd = "ssh -t -t %s%s -- '%s'" % (user, machine, cmd)
 
-    pty_fds = pty.openpty()
     p = subprocess.Popen(cmd, stdout=subprocess.PIPE,
                          stderr=subprocess.PIPE,
-                         stdin=pty_fds[0], shell=True)
+                         shell=True)
 
     full_stdout = ""
     full_stderr = ""
@@ -135,8 +133,6 @@ class CommandExecuter:
         break
 
     p.wait()
-    os.close(pty_fds[0])
-    os.close(pty_fds[1])
     if return_output:
       return (p.returncode, full_stdout, full_stderr)
     return p.returncode
diff --git a/utils/no_pseudo_terminal_test.py b/utils/no_pseudo_terminal_test.py
new file mode 100644
index 00000000..c4c3c43e
--- /dev/null
+++ b/utils/no_pseudo_terminal_test.py
@@ -0,0 +1,49 @@
+"""Test to ensure we're not touching /dev/ptmx when running commands."""
+
+import os
+import subprocess
+import tempfile
+import time
+import unittest
+from utils import command_executer
+
+
+class NoPsuedoTerminalTest(unittest.TestCase):
+
+  STRACE_TIMEOUT = 10
+
+  def _AttachStraceToSelf(self, output_file):
+    """Attaches strace to the current process."""
+    args = ["strace", "-o", output_file, "-p", str(os.getpid())]
+    print args
+    self._strace_process = subprocess.Popen(args)
+    # Wait until we see some activity.
+    start_time = time.time()
+    while time.time() - start_time < self.STRACE_TIMEOUT:
+      if os.path.isfile(output_file) and open(output_file).read(1):
+        return True
+      time.sleep(1)
+    return False
+
+  def _KillStraceProcess(self):
+    """Kills strace that was started by _AttachStraceToSelf()."""
+    self._strace_process.terminate()
+    self._strace_process.wait()
+    return True
+
+  def testNoPseudoTerminalWhenRunningCommand(self):
+    """Test to make sure we're not touching /dev/ptmx when running commands."""
+    temp_file = tempfile.mktemp()
+    self.assertTrue(self._AttachStraceToSelf(temp_file))
+
+    ce = command_executer.GetCommandExecuter()
+    ce.RunCommand("echo")
+
+    self.assertTrue(self._KillStraceProcess())
+
+    strace_contents = open(temp_file).read()
+    self.assertFalse("/dev/ptmx" in strace_contents)
+
+
+if __name__ == '__main__':
+  unittest.main()