Remove the thread and queue between execReadlines and the process it
starts. Instead of reading child process output into an unbounded
buffer, have the child block until we are ready to read input, and kill
the child when we are done reading input. Return an iterator object
instead of a generator so that we can kill the child on __del__.
Add a test for whether the child is killed when the iterator is
unreferenced.
---
pyanaconda/iutil.py | 61 ++++++++++--------
tests/pyanaconda_tests/iutil_test.py | 118 +++++++++++++++++++++--------------
2 files changed, 108 insertions(+), 71 deletions(-)
diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py
index 999b4ec..97b9f35 100644
--- a/pyanaconda/iutil.py
+++ b/pyanaconda/iutil.py
@@ -31,8 +31,6 @@ import string
import tempfile
import types
import re
-from threading import Thread
-from Queue import Queue, Empty
from urllib import quote, unquote
import gettext
import signal
@@ -266,13 +264,42 @@ def execReadlines(command, argv, stdin=None, root='/',
env_prune=None):
:param env_prune: environment variable to remove before execution
Output from the file is not logged to program.log
- This returns a generator with the lines from the command until it has finished
+ This returns an iterator with the lines from the command until it has finished
"""
- # Return the lines from stdout via a Queue
- def queue_lines(out, queue):
- for line in iter(out.readline, b''):
- queue.put(line.strip())
- out.close()
+
+ class ExecLineReader(object):
+ """Iterator class for returning lines from a process and cleaning
+ up the process when the output is no longer needed.
+ """
+
+ def __init__(self, proc, argv):
+ self._proc = proc
+ self._argv = argv
+
+ def __iter__(self):
+ return self
+
+ def __del__(self):
+ # See if the process is still running
+ if self._proc.poll() is None:
+ # Stop the process and ignore any problems that might arise
+ try:
+ self._proc.terminate()
+ except OSError:
+ pass
+
+ def next(self):
+ # Read the next line, blocking if a line is not yet available
+ line = self._proc.stdout.readline()
+ if line == '':
+ # Output finished, check for the process dying unexpectedly
+ # and stop the iteration
+ if self._proc.poll() is not None:
+ if os.WIFSIGNALED(self._proc.returncode):
+ raise OSError("process '%s' was killed" %
self._argv)
+ raise StopIteration
+
+ return line.strip()
argv = [command] + argv
@@ -283,23 +310,7 @@ def execReadlines(command, argv, stdin=None, root='/',
env_prune=None):
program_log.error("Error running %s: %s", argv[0], e.strerror)
raise
- q = Queue()
- t = Thread(target=queue_lines, args=(proc.stdout, q))
- t.daemon = True # thread dies with the program
- t.start()
-
- while True:
- try:
- line = q.get(timeout=.1)
- yield line
- q.task_done()
- except Empty:
- if proc.poll() is not None:
- if os.WIFSIGNALED(proc.returncode):
- raise OSError("process '%s' was killed" % argv)
- break
- q.join()
-
+ return ExecLineReader(proc, argv)
## Run a shell.
def execConsole():
diff --git a/tests/pyanaconda_tests/iutil_test.py b/tests/pyanaconda_tests/iutil_test.py
index b048f9c..4d28606 100644
--- a/tests/pyanaconda_tests/iutil_test.py
+++ b/tests/pyanaconda_tests/iutil_test.py
@@ -21,7 +21,6 @@
from pyanaconda import iutil
import unittest
-import types
import os
import tempfile
import signal
@@ -103,12 +102,10 @@ class RunProgramTests(unittest.TestCase):
# test some lines are returned
self.assertGreater(len(list(iutil.execReadlines("ls",
["--help"]))), 0)
- # check that it always returns a generator for both
+ # check that it always returns an iterator for both
# if there is some output and if there isn't any
- self.assertIsInstance(iutil.execReadlines("ls", ["--help"]),
- types.GeneratorType)
- self.assertIsInstance(iutil.execReadlines("true", []),
- types.GeneratorType)
+ self.assertTrue(hasattr(iutil.execReadlines("ls",
["--help"]), "__iter__"))
+ self.assertTrue(hasattr(iutil.execReadlines("true", []),
"__iter__"))
def exec_readlines_test_normal_output(self):
"""Test the output of execReadlines."""
@@ -124,11 +121,11 @@ exit 0
testscript.flush()
with timer(5):
- rl_generator = iutil.execReadlines("/bin/sh",
[testscript.name])
- self.assertEqual(rl_generator.next(), "one")
- self.assertEqual(rl_generator.next(), "two")
- self.assertEqual(rl_generator.next(), "three")
- self.assertRaises(StopIteration, rl_generator.next)
+ rl_iterator = iutil.execReadlines("/bin/sh",
[testscript.name])
+ self.assertEqual(rl_iterator.next(), "one")
+ self.assertEqual(rl_iterator.next(), "two")
+ self.assertEqual(rl_iterator.next(), "three")
+ self.assertRaises(StopIteration, rl_iterator.next)
# Test output with no end of line
with tempfile.NamedTemporaryFile() as testscript:
@@ -141,11 +138,11 @@ exit 0
testscript.flush()
with timer(5):
- rl_generator = iutil.execReadlines("/bin/sh",
[testscript.name])
- self.assertEqual(rl_generator.next(), "one")
- self.assertEqual(rl_generator.next(), "two")
- self.assertEqual(rl_generator.next(), "three")
- self.assertRaises(StopIteration, rl_generator.next)
+ rl_iterator = iutil.execReadlines("/bin/sh",
[testscript.name])
+ self.assertEqual(rl_iterator.next(), "one")
+ self.assertEqual(rl_iterator.next(), "two")
+ self.assertEqual(rl_iterator.next(), "three")
+ self.assertRaises(StopIteration, rl_iterator.next)
def exec_readlines_test_exits(self):
"""Test execReadlines in different child exit
situations."""
@@ -163,11 +160,11 @@ exit 1
testscript.flush()
with timer(5):
- rl_generator = iutil.execReadlines("/bin/sh",
[testscript.name])
- self.assertEqual(rl_generator.next(), "one")
- self.assertEqual(rl_generator.next(), "two")
- self.assertEqual(rl_generator.next(), "three")
- self.assertRaises(OSError, rl_generator.next)
+ rl_iterator = iutil.execReadlines("/bin/sh",
[testscript.name])
+ self.assertEqual(rl_iterator.next(), "one")
+ self.assertEqual(rl_iterator.next(), "two")
+ self.assertEqual(rl_iterator.next(), "three")
+ self.assertRaises(OSError, rl_iterator.next)
# Test exit on signal
with tempfile.NamedTemporaryFile() as testscript:
@@ -180,11 +177,11 @@ kill -TERM $$
testscript.flush()
with timer(5):
- rl_generator = iutil.execReadlines("/bin/sh",
[testscript.name])
- self.assertEqual(rl_generator.next(), "one")
- self.assertEqual(rl_generator.next(), "two")
- self.assertEqual(rl_generator.next(), "three")
- self.assertRaises(OSError, rl_generator.next)
+ rl_iterator = iutil.execReadlines("/bin/sh",
[testscript.name])
+ self.assertEqual(rl_iterator.next(), "one")
+ self.assertEqual(rl_iterator.next(), "two")
+ self.assertEqual(rl_iterator.next(), "three")
+ self.assertRaises(OSError, rl_iterator.next)
# Repeat the above two tests, but exit before a final newline
with tempfile.NamedTemporaryFile() as testscript:
@@ -197,11 +194,11 @@ exit 1
testscript.flush()
with timer(5):
- rl_generator = iutil.execReadlines("/bin/sh",
[testscript.name])
- self.assertEqual(rl_generator.next(), "one")
- self.assertEqual(rl_generator.next(), "two")
- self.assertEqual(rl_generator.next(), "three")
- self.assertRaises(OSError, rl_generator.next)
+ rl_iterator = iutil.execReadlines("/bin/sh",
[testscript.name])
+ self.assertEqual(rl_iterator.next(), "one")
+ self.assertEqual(rl_iterator.next(), "two")
+ self.assertEqual(rl_iterator.next(), "three")
+ self.assertRaises(OSError, rl_iterator.next)
with tempfile.NamedTemporaryFile() as testscript:
testscript.write("""#!/bin/sh
@@ -213,11 +210,11 @@ kill -TERM $$
testscript.flush()
with timer(5):
- rl_generator = iutil.execReadlines("/bin/sh",
[testscript.name])
- self.assertEqual(rl_generator.next(), "one")
- self.assertEqual(rl_generator.next(), "two")
- self.assertEqual(rl_generator.next(), "three")
- self.assertRaises(OSError, rl_generator.next)
+ rl_iterator = iutil.execReadlines("/bin/sh",
[testscript.name])
+ self.assertEqual(rl_iterator.next(), "one")
+ self.assertEqual(rl_iterator.next(), "two")
+ self.assertEqual(rl_iterator.next(), "three")
+ self.assertRaises(OSError, rl_iterator.next)
def exec_readlines_test_signals(self):
"""Test execReadlines and signal receipt."""
@@ -236,11 +233,11 @@ exit 0
testscript.flush()
with timer(5):
- rl_generator = iutil.execReadlines("/bin/sh",
[testscript.name])
- self.assertEqual(rl_generator.next(), "one")
- self.assertEqual(rl_generator.next(), "two")
- self.assertEqual(rl_generator.next(), "three")
- self.assertRaises(StopIteration, rl_generator.next)
+ rl_iterator = iutil.execReadlines("/bin/sh",
[testscript.name])
+ self.assertEqual(rl_iterator.next(), "one")
+ self.assertEqual(rl_iterator.next(), "two")
+ self.assertEqual(rl_iterator.next(), "three")
+ self.assertRaises(StopIteration, rl_iterator.next)
finally:
signal.signal(signal.SIGHUP, old_HUP_handler)
@@ -260,11 +257,11 @@ exit 0
testscript.flush()
with timer(5):
- rl_generator = iutil.execReadlines("/bin/sh",
[testscript.name])
- self.assertEqual(rl_generator.next(), "one")
- self.assertEqual(rl_generator.next(), "two")
- self.assertEqual(rl_generator.next(), "three")
- self.assertRaises(StopIteration, rl_generator.next)
+ rl_iterator = iutil.execReadlines("/bin/sh",
[testscript.name])
+ self.assertEqual(rl_iterator.next(), "one")
+ self.assertEqual(rl_iterator.next(), "two")
+ self.assertEqual(rl_iterator.next(), "three")
+ self.assertRaises(StopIteration, rl_iterator.next)
finally:
signal.signal(signal.SIGHUP, old_HUP_handler)
@@ -339,6 +336,35 @@ while true ; do sleep 1 ; done
proc.communicate()
self.assertEqual(proc.returncode, -(signal.SIGTERM))
+ def exec_readlines_auto_kill_test(self):
+ """Test execReadlines with reading only part of the
output"""
+
+ with tempfile.NamedTemporaryFile() as testscript:
+ testscript.write("""#!/bin/sh
+# Output forever
+while true; do
+echo hey
+done
+""")
+ testscript.flush()
+
+ with timer(5):
+ rl_iterator = iutil.execReadlines("/bin/sh",
[testscript.name])
+
+ # Save the process context
+ proc = rl_iterator._proc
+
+ # Read two lines worth
+ self.assertEqual(rl_iterator.next(), "hey")
+ self.assertEqual(rl_iterator.next(), "hey")
+
+ # Delete the iterator and wait for the process to be killed
+ del rl_iterator
+ proc.communicate()
+
+ # Check that the process is gone
+ self.assertIsNotNone(proc.poll())
+
class MiscTests(unittest.TestCase):
def get_dir_size_test(self):
"""Test the getDirSize."""
--
1.9.3