How about just check timeout by  ”proc.returncode == -9“, not sure we can kill the subprocess manually.

The follow is an Example:

import subprocess
from threading import Timer

def run_command(cmd, timeout=None):
    proc = None
    timeout_flag = False

    try:
        proc = subprocess.Popen(cmd, stdout=subprocess.PIPE,
                                stderr=subprocess.PIPE)
        if timeout is not None:
            timer = Timer(timeout, lambda proc: proc.kill(), [proc])
            timer.start()

        out, error = proc.communicate()
        print "Run command '%s'" % " ".join(cmd)

        if out or error:
            print "out:\n %s\nerror:\n %s" % (out, error)
        if proc.returncode == -9:
            timeout_flag = True
            print "process is killed by signal.SIGKILL for timeout %s seconds" % timeout
        if timeout is not None:
            timer.cancel()
        return out, error, proc.returncode, timeout_flag
    except Exception as e:
        msg = "Failed to run command: %s." % " ".join(cmd)
        print "%s\n  %s" % (msg, e)
        return None, None, None, timeout_flag


On 01/10/2014 04:16 PM, Royce Lv wrote:

On 2014年01月10日 09:00, shaohef@linux.vnet.ibm.com wrote:

From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>

We need a common function to execute shell command.
We also need timeout when execute shell command.

A threading.Timer is used to send signal.SIGKILL to kill the command
when timeout.

Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com>
Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
---
  src/kimchi/utils.py | 39 +++++++++++++++++++++++++++++++++++++++
  1 file changed, 39 insertions(+)

diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py
index af245c6..94097ad 100644
--- a/src/kimchi/utils.py
+++ b/src/kimchi/utils.py
@@ -23,6 +23,7 @@

  import cherrypy
  import os
+import subprocess
  import urllib2


@@ -30,6 +31,7 @@ from cherrypy.lib.reprconf import Parser


  from kimchi import config
+from threading import Timer


  kimchi_log = cherrypy.log.error_log
@@ -96,3 +98,40 @@ def check_url_path(path):
          return False

      return True
+
+
+def run_command(cmd, timeout=None):
+    def kill_proc(proc, timeout_flag):
+        timeout_flag[0] = True
+        proc.kill()
+
+    proc = None
+    timer = None
+    timeout_flag = [False]
+
+    try:
+        proc = subprocess.Popen(cmd, stdout=subprocess.PIPE,
+                                stderr=subprocess.PIPE)
+        if timeout is not None:
+            timer = Timer(timeout, kill_proc, [proc, timeout_flag])
+            timer.start()

I like the idea of using timer, I think the only thing we need to take care is check timeout happens and report a specific error.
Or when we kill the proc and it is blocked, the proc.communicate() error will not be 'timeout happens'

+
+        out, error = proc.communicate()
+        kimchi_log.debug("Run command '%s'", " ".join(cmd))
+
+        if out or error:
+            kimchi_log.debug("out:\n %s\nerror:\n %s", out, error)
+
+        return out, error, proc.returncode, timeout_flag[0]
+    except Exception as e:
+        msg = "Failed to run command: %s." % " ".join(cmd)
+        msg = msg if proc is None else msg + "\n  error code: %s."
+        kimchi_log.error("%s\n  %s", msg, e)
+
+        if proc:
+            return out, error, proc.returncode, timeout_flag[0]
+        else:
+            return None, None, None, timeout_flag[0]

+    finally:
+        if timer and not timeout_flag[0]:
+            timer.cancel()

-- 
Thanks and best regards!

Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com>
IBM Linux Technology Center