From fa371eb88b45113e4e7c63972f6ac141e816c6b0 Mon Sep 17 00:00:00 2001 From: Mitar Date: Sun, 6 May 2012 14:35:29 +0200 Subject: [PATCH 1/2] Communication over pipe made more robust. --- tracext/git/PyGIT.py | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/tracext/git/PyGIT.py b/tracext/git/PyGIT.py index 8e8f7d3..91d638b 100644 --- a/tracext/git/PyGIT.py +++ b/tracext/git/PyGIT.py @@ -17,6 +17,7 @@ from contextlib import contextmanager import cStringIO import codecs +from trac.util import TracError __all__ = ["git_version", "GitError", "GitErrorSha", "Storage", "StorageFactory"] @@ -270,6 +271,7 @@ def __init__(self, git_dir, log, git_bin='git', git_fs_encoding=None): def __del__(self): if self.__cat_file_pipe is not None: self.__cat_file_pipe.stdin.close() + self.__cat_file_pipe.terminate() self.__cat_file_pipe.wait() # @@ -495,15 +497,29 @@ def cat_file(self, kind, sha): if self.__cat_file_pipe is None: self.__cat_file_pipe = self.repo.cat_file_batch() - self.__cat_file_pipe.stdin.write(sha + '\n') - self.__cat_file_pipe.stdin.flush() - _sha, _type, _size = self.__cat_file_pipe.stdout.readline().split() + try: + self.__cat_file_pipe.stdin.write(sha + '\n') + self.__cat_file_pipe.stdin.flush() + + split_stdout_line = self.__cat_file_pipe.stdout.readline().split() + if len(split_stdout_line) != 3: + raise TracError("internal error (could not split line properly %s)" % (split_stdout_line,)) + + _sha, _type, _size = split_stdout_line - if _type != kind: - raise TracError("internal error (got unexpected object kind '%s')" % k) + if _type != kind: + raise TracError("internal error (got unexpected object kind '%s', expected '%s')" % (_type, kind)) - size = int(_size) - return self.__cat_file_pipe.stdout.read(size + 1)[:size] + size = int(_size) + return self.__cat_file_pipe.stdout.read(size + 1)[:size] + except: + # There was an error, we should close the pipe to get to a consistent state + # (Otherwise it happens that next time we call cat_file we get payload from previous call) + self.logger.debug("closing cat_file pipe") + self.__cat_file_pipe.stdin.close() + self.__cat_file_pipe.terminate() + self.__cat_file_pipe.wait() + self.__cat_file_pipe = None def verifyrev(self, rev): "verify/lookup given revision object and return a sha id or None if lookup failed" From e64278ded149661ea1186b74593156f6425fe843 Mon Sep 17 00:00:00 2001 From: Mitar Date: Sun, 6 May 2012 15:36:14 +0200 Subject: [PATCH 2/2] Use locking when reading from a pipe. --- tracext/git/PyGIT.py | 62 +++++++++++++++++++++++--------------------- 1 file changed, 33 insertions(+), 29 deletions(-) diff --git a/tracext/git/PyGIT.py b/tracext/git/PyGIT.py index 91d638b..2484969 100644 --- a/tracext/git/PyGIT.py +++ b/tracext/git/PyGIT.py @@ -267,12 +267,14 @@ def __init__(self, git_dir, log, git_bin='git', git_fs_encoding=None): self.__commit_msg_lock = Lock() self.__cat_file_pipe = None + self.__cat_file_pipe_lock = Lock() def __del__(self): - if self.__cat_file_pipe is not None: - self.__cat_file_pipe.stdin.close() - self.__cat_file_pipe.terminate() - self.__cat_file_pipe.wait() + with self.__cat_file_pipe_lock: + if self.__cat_file_pipe is not None: + self.__cat_file_pipe.stdin.close() + self.__cat_file_pipe.terminate() + self.__cat_file_pipe.wait() # # cache handling @@ -494,32 +496,34 @@ def head(self): return self.verifyrev("HEAD") def cat_file(self, kind, sha): - if self.__cat_file_pipe is None: - self.__cat_file_pipe = self.repo.cat_file_batch() + with self.__cat_file_pipe_lock: + if self.__cat_file_pipe is None: + self.__cat_file_pipe = self.repo.cat_file_batch() - try: - self.__cat_file_pipe.stdin.write(sha + '\n') - self.__cat_file_pipe.stdin.flush() - - split_stdout_line = self.__cat_file_pipe.stdout.readline().split() - if len(split_stdout_line) != 3: - raise TracError("internal error (could not split line properly %s)" % (split_stdout_line,)) - - _sha, _type, _size = split_stdout_line - - if _type != kind: - raise TracError("internal error (got unexpected object kind '%s', expected '%s')" % (_type, kind)) - - size = int(_size) - return self.__cat_file_pipe.stdout.read(size + 1)[:size] - except: - # There was an error, we should close the pipe to get to a consistent state - # (Otherwise it happens that next time we call cat_file we get payload from previous call) - self.logger.debug("closing cat_file pipe") - self.__cat_file_pipe.stdin.close() - self.__cat_file_pipe.terminate() - self.__cat_file_pipe.wait() - self.__cat_file_pipe = None + try: + self.__cat_file_pipe.stdin.write(sha + '\n') + self.__cat_file_pipe.stdin.flush() + + split_stdout_line = self.__cat_file_pipe.stdout.readline().split() + if len(split_stdout_line) != 3: + raise TracError("internal error (could not split line properly %s)" % (split_stdout_line,)) + + _sha, _type, _size = split_stdout_line + + if _type != kind: + raise TracError("internal error (got unexpected object kind '%s', expected '%s')" % (_type, kind)) + + size = int(_size) + return self.__cat_file_pipe.stdout.read(size + 1)[:size] + except: + # There was an error, we should close the pipe to get to a consistent state + # (Otherwise it happens that next time we call cat_file we get payload from previous call) + self.logger.debug("closing cat_file pipe") + self.__cat_file_pipe.stdin.close() + self.__cat_file_pipe.terminate() + self.__cat_file_pipe.wait() + self.__cat_file_pipe = None + raise def verifyrev(self, rev): "verify/lookup given revision object and return a sha id or None if lookup failed"