Thursday 14 January 2010

Where's a plumber when you need one?

Assertion: On Win32, there's no point bothering with subprocess.PIPE -- just use tempfile.TemporaryFile instead.

Context: You create a Popen(cmd, stdout=PIPE, stderr=PIPE), and you let it run (with a timeout); sometimes it completes successfully, which is cool, and sometimes it doesn't, in which case you read stdout and stderr and try to figure out what went wrong. This is all fine and dandy until one day you add a *little* bit more logging to the tool you're calling, and it suddenly wedges forever.

Explanation: Of course, this is because you've filled up some buffer, which you should have been periodically emptying. However, you can't just select() and read one byte at a time, because select doesn't work with pipes on Win32; you can't just read() what's there, because it blocks and stops the timeout from working; you don't want to spin off another thread to do your reading, because that involves tedious extra code and feels like killing a fly with a sledgehammer; and you don't want to screw around with readline() because that also involves tedious bookkeeping and extra code.

Solution: So, just do the following (warning, coded from memory):


from subprocess import Popen
from tempfile import TemporaryFile
from time import time, sleep

def assert_runs(cmd, timeout=10):
out = TemporaryFile()
err = TemporaryFile()
end_time = time() + timeout
process = Popen(cmd, stdout=out, stderr=err)
while process.poll() is None:
sleep(0.1)
if time() > end_time:
process.terminate()
if process.returncode != 0:
raise AssertionError('%s FAILED (%s)\nstdout:\n%s\nstderr:\n%s' % (
cmd, process.returncode, out.read(), err.read()))


Indeed, it's icky to fill up your hard disk rather than some internal buffer, but you can get a lot more done before you run out of HD space. Now, this surely feels nasty, but IMO it's slightly less nasty (or, at least, less code) than anything else I've tried. Hopefully you, gentle reader, have an infinitely superior solution that you will detail in the comments. Surprise me!

Please note that "Don't use Windows, har har", and variants thereof, fail to qualify as "surprising" ;-).

8 comments:

Unknown said...

Maybe something like this (written directly in the comment textbox – untested and may contain typos):
process = Popen(cmd, stdout=PIPE, stderr=PIPE)
timer = threading.Timer(timeout, process.terminate)
timer.start()
out, err = process.communicate()
timer.cancel()
?
It uses three threads, but the code is short :)

Pete said...

Nice. I wonder if it works for StringIo. Then you can just worry about filling your memory, rather than temp disk space.

Jean-Paul Calderone said...

Probably not what you're interested in, but just in case - Twisted's process support doesn't run into this issue. It actually reads from all of the pipes as necessary.

Sidnei da Silva said...

I am almost sure you can use StringIO. Been here, done that. Just my memory doesn't help me reminding the details. :)

william said...

Thank you all for useful comments -- I especially like the threaded termination, but even if you don't use that I agree that it's much nicer to use StringIo rather than a real file.

Marius Gedminas said...

This is actually not Windows-specific: if you try to synchronously read from stdout and stderr in the same thread, you risk hanging on Unix too.

That's why subprocess.Popen has the communicate() method. The docs even warn about it:

"Warning Use communicate() rather than .stdin.write, .stdout.read or .stderr.read to avoid deadlocks due to any of the other OS pipe buffers filling up and blocking the child process."

-- http://docs.python.org/library/subprocess.html

jonesalbert said...

Plumbing as a profession can be quite lucrative considering the huge demand for expert plumbers in the domestic as well as office setups.

Plumber South Yarra

Ned Polian said...

Once I thought about things like: why such information is for free here? Because when you write a book then at least on selling a book you get a percentage. Thank you and good luck on informing people more about it! plumbing contractor