Author: futatuki Date: Fri Oct 11 09:36:56 2019 New Revision: 1868285 URL: http://svn.apache.org/viewvc?rev=1868285&view=rev Log: On branch swig-py3: follow-up to 1867779: Use wrapper object to clean up for stdout pipe of sub process
* subversion/bindings/swig/python/svn/fs.py: (_PopenStdoutWrapper): New class (FileDiff.procs): Removed (FileDiff.get_pipe): - Don't hold subprocess.Popen object. - Return _PopenStdoutWrappper object instead of subprocess.Popen.stdout. (FileDiff.__del__): Remove clean up code for subproces.Popen object created in FileDiff.get_pipe. Modified: subversion/branches/swig-py3/subversion/bindings/swig/python/svn/fs.py Modified: subversion/branches/swig-py3/subversion/bindings/swig/python/svn/fs.py URL: http://svn.apache.org/viewvc/subversion/branches/swig-py3/subversion/bindings/swig/python/svn/fs.py?rev=1868285&r1=1868284&r2=1868285&view=diff ============================================================================== --- subversion/branches/swig-py3/subversion/bindings/swig/python/svn/fs.py (original) +++ subversion/branches/swig-py3/subversion/bindings/swig/python/svn/fs.py Fri Oct 11 09:36:56 2019 @@ -52,6 +52,26 @@ def entries(root, path, pool=None): e[name] = dirent_t_id_get(entry) return e +class _PopenStdoutWrapper(object): + "Private wrapper object of _subprocess.Popen.stdout to clean up sub process" + def __init__(self, pobject): + self._pobject = pobject + def __getattr__(self, name): + return getattr(self._pobject.stdout, name) + def close(self): + self._pobject.stdout.close() + if self._pobject.poll() is None: + self._pobject.terminate() + def __del__(self): + if not self.closed: + self.close() + if self._pobject.poll() is None: + self._pobject.terminate() + if _sys.hexversion >= 0x030300F0: + try: + self._pobject.wait(10) + except _subprocess.TimeoutExpired: + self._pobject.kill() class FileDiff: def __init__(self, root1, path1, root2, path2, pool=None, diffoptions=[]): @@ -60,7 +80,6 @@ class FileDiff: self.tempfile1 = None self.tempfile2 = None self.difftemp = None - self.procs = [] self.root1 = root1 self.path1 = path1 @@ -125,8 +144,7 @@ class FileDiff: # open the pipe, and return the file object for reading from the child. p = _subprocess.Popen(cmd, stdout=_subprocess.PIPE, bufsize=-1, close_fds=_sys.platform != "win32") - self.procs.append(p) - return p.stdout + return _PopenStdoutWrapper(p) else: if self.difftemp is None: @@ -152,16 +170,6 @@ class FileDiff: return builtins.open(self.difftemp, "rb") def __del__(self): - # subprocess created by self.get_pipe() should be terminated only if - # its stdout is already closed - for proc in self.procs: - if proc.poll() is None and proc.stdout.closed: - proc.terminate() - if _sys.hexversion >= 0x030300F0: - try: - proc.wait(10) - except subprocess.TimeoutExpired: - proc.kill() # it seems that sometimes the files are deleted, so just ignore any # failures trying to remove them for tmpfile in [self.tempfile1, self.tempfile2, self.difftemp]: