Fixed leaking of file descriptors in utils.run
The run function didn't close fds in the parent process. This adds
code to ensure that all code paths close the fds. Code repeatedly
executed runs before (e.g. host.wait_up) could exhaust the
file descriptor pool.
Though this patch looks large it mostly consists of indenting a
large chunk of code to make it part of a try/finally.
Signed-off-by: Ryan Stutsman <stutsman@google.com>
git-svn-id: http://test.kernel.org/svn/autotest/trunk@604 592f7852-d20e-0410-864c-8624ca9c26a4
diff --git a/server/utils.py b/server/utils.py
index 47e0927..afedfce 100644
--- a/server/utils.py
+++ b/server/utils.py
@@ -158,68 +158,76 @@
sp= subprocess.Popen(command, stdout=subprocess.PIPE,
stderr=subprocess.PIPE, close_fds=True, shell=True,
executable="/bin/bash")
-
- start_time= time.time()
- if timeout:
- stop_time= start_time + timeout
- time_left= stop_time - time.time()
- while time_left > 0:
- # select will return when stdout is ready
- # (including when it is EOF, that is the
- # process has terminated).
- (retval, tmp, tmp) = select.select(
- [sp.stdout], [], [], time_left)
- if len(retval):
- # os.read() has to be used instead of
- # sp.stdout.read() which will
- # otherwise block
- result.stdout += os.read(
- sp.stdout.fileno(), 1024)
-
- (pid, exit_status_indication) = os.waitpid(
- sp.pid, os.WNOHANG)
- if pid:
- stop_time= time.time()
+
+ try:
+ # We are holding ends to stdin, stdout pipes
+ # hence we need to be sure to close those fds no mater what
+ start_time= time.time()
+ if timeout:
+ stop_time= start_time + timeout
time_left= stop_time - time.time()
-
- # the process has not terminated within timeout,
- # kill it via an escalating series of signals.
- if not pid:
- signal_queue = [signal.SIGTERM, signal.SIGKILL]
- for sig in signal_queue:
- try:
- os.kill(sp.pid, sig)
- # handle race condition in which
- # process died before we could kill it.
- except OSError:
- pass
-
- for i in range(5):
- (pid, exit_status_indication
- ) = os.waitpid(sp.pid,
- os.WNOHANG)
+ while time_left > 0:
+ # select will return when stdout is ready
+ # (including when it is EOF, that is the
+ # process has terminated).
+ (retval, tmp, tmp) = select.select(
+ [sp.stdout], [], [], time_left)
+ if len(retval):
+ # os.read() has to be used instead of
+ # sp.stdout.read() which will
+ # otherwise block
+ result.stdout += os.read(
+ sp.stdout.fileno(), 1024)
+
+ (pid, exit_status_indication) = os.waitpid(
+ sp.pid, os.WNOHANG)
+ if pid:
+ stop_time= time.time()
+ time_left= stop_time - time.time()
+
+ # the process has not terminated within timeout,
+ # kill it via an escalating series of signals.
+ if not pid:
+ signal_queue = [signal.SIGTERM, signal.SIGKILL]
+ for sig in signal_queue:
+ try:
+ os.kill(sp.pid, sig)
+ # handle race condition in which
+ # process died before we could kill it.
+ except OSError:
+ pass
+
+ for i in range(5):
+ (pid, exit_status_indication
+ ) = os.waitpid(sp.pid,
+ os.WNOHANG)
+ if pid:
+ break
+ else:
+ time.sleep(1)
if pid:
break
- else:
- time.sleep(1)
- if pid:
- break
- else:
- exit_status_indication = os.waitpid(sp.pid, 0)[1]
-
- result.duration = time.time() - start_time
- result.aborted = exit_status_indication & 127
- if result.aborted:
- result.exit_status= None
- else:
- result.exit_status= exit_status_indication / 256
- result.stdout += sp.stdout.read()
- result.stderr = sp.stderr.read()
+ else:
+ exit_status_indication = os.waitpid(sp.pid, 0)[1]
+
+ result.duration = time.time() - start_time
+ result.aborted = exit_status_indication & 127
+ if result.aborted:
+ result.exit_status= None
+ else:
+ result.exit_status= exit_status_indication / 256
+ result.stdout += sp.stdout.read()
+ result.stderr = sp.stderr.read()
+
+ finally:
+ # close our ends of the pipes to the sp no matter what
+ sp.stdout.close()
+ sp.stderr.close()
if not ignore_status and result.exit_status > 0:
raise errors.AutoservRunError("command execution error",
result)
-
+
return result