From ba944fc852daed09cb095a84c4f80c6beaf8c663 Mon Sep 17 00:00:00 2001 From: Martin Dorey Date: Mon, 5 Nov 2018 15:11:27 -0800 Subject: [PATCH 1/2] subprocessio.py: Apply a native English speaker's guesses to those squiggly red underlines in the comments that appear accidental. --- subprocessio.py | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/subprocessio.py b/subprocessio.py index 409b4b0..5430f0a 100644 --- a/subprocessio.py +++ b/subprocessio.py @@ -1,9 +1,9 @@ #!/usr/bin/env python ''' Module provides a class allowing to wrap communication over subprocess.Popen -input, output, error streams into a meaningfull, non-blocking, concurrent stream +input, output, error streams into a meaningful, non-blocking, concurrent stream processor exposing the output data as an iterator fitting to be a return value -passed by a WSGI applicaiton to a WSGI server per PEP 3333. +passed by a WSGI application to a WSGI server per PEP 3333. Copyright (c) 2011 Daniel Dotsenko @@ -219,7 +219,7 @@ def reading_paused(self): @property def done_reading_event(self): ''' - Done_reding does not mean that the iterator's buffer is empty. + Done_reading does not mean that the iterator's buffer is empty. Iterator might have done reading from underlying source, but the read chunks might still be available for serving through .next() method. @@ -229,11 +229,11 @@ def done_reading_event(self): @property def done_reading(self): ''' - Done_reding does not mean that the iterator's buffer is empty. + Done_reading does not mean that the iterator's buffer is empty. Iterator might have done reading from underlying source, but the read chunks might still be available for serving through .next() method. - @return An Bool value. + @return A Bool value. ''' return self.worker.EOF.is_set() @@ -242,15 +242,15 @@ def length(self): ''' returns int. - This is the lenght of the que of chunks, not the length of + This is the length of the que of chunks, not the length of the combined contents in those chunks. __len__() cannot be meaningfully implemented because this - reader is just flying throuh a bottomless pit content and - can only know the lenght of what it already saw. + reader is just flying through a bottomless pit of content and + can only know the length of what it already saw. If __len__() on WSGI server per PEP 3333 returns a value, - the responce's length will be set to that. In order not to + the response's length will be set to that. In order not to confuse WSGI PEP3333 servers, we will not implement __len__ at all. ''' @@ -285,11 +285,11 @@ class SubprocessIOChunker(): does not block the parallel inpipe reading occurring parallel thread.) The purpose of the object is to allow us to wrap subprocess interactions into - and interable that can be passed to a WSGI server as the application's return + an iterable that can be passed to a WSGI server as the application's return value. Because of stream-processing-ability, WSGI does not have to read ALL of the subprocess's output and buffer it, before handing it to WSGI server for HTTP response. Instead, the class initializer reads just a bit of the stream - to figure out if error ocurred or likely to occur and if not, just hands the + to figure out if error occurred or likely to occur and if not, just hands the further iteration over subprocess output to the server for completion of HTTP response. @@ -391,4 +391,3 @@ def close(self): def __del__(self): self.close() - From 75bbfae21d3fa72bede2510bd511a80acd54b062 Mon Sep 17 00:00:00 2001 From: Martin Dorey Date: Thu, 3 Jan 2019 11:50:44 -0800 Subject: [PATCH 2/2] subprocessio.py: Fix both of the pipe leaks described in https://github.com/dvdotsenko/git_http_backend.py/issues/19, the originally motivating one causing "Too many open files" errors in eg RhodeCode, with the fix from Mads Kiilerich at https://bitbucket.org/conservancy/kallithea/issues/32, and the one Uma Parvathappa found when write(2) gets EPIPE. --- subprocessio.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/subprocessio.py b/subprocessio.py index 5430f0a..916b1a8 100644 --- a/subprocessio.py +++ b/subprocessio.py @@ -61,15 +61,17 @@ def __init__(self, source): def run(self): t = self.writeiface - if self.bytes: - os.write(t, self.bytes) - else: - s = self.source - b = s.read(4096) - while b: - os.write(t, b) + try: + if self.bytes: + os.write(t, self.bytes) + else: + s = self.source b = s.read(4096) - os.close(t) + while b: + os.write(t, b) + b = s.read(4096) + finally: + os.close(t) @property def output(self): @@ -362,6 +364,7 @@ def __init__(self, cmd, inputstream = None, buffer_size = 65536, chunk_size = 40 self.process = _p self.output = bg_out self.error = bg_err + self.inputstream = inputstream def __iter__(self): return self @@ -388,6 +391,10 @@ def close(self): self.error.close() except: pass + try: + os.close(self.inputstream) + except: + pass def __del__(self): self.close()