Made improvements to assure test-cases don't leak file handles

At least leakage is considerably reduced.
Additionally, a test-case was added which triggers failure if auto-disposal
of resources wouldn't work.

Fixes #60

7 files changed, 57 insertions, 6 deletions

diff --git a/.travis.yml b/.travis.yml
index 4f77dbca..112df6cf 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -25,6 +25,9 @@ install:
   - git config --global user.email "travis@ci.com"
   - git config --global user.name "Travis Runner"
 script: 
+  # Make sure we limit open handles to see if we are leaking them
+  - ulimit -n 64
+  - ulimit -n
   - nosetests -v --with-coverage
   - flake8
 after_success:
diff --git a/git/cmd.py b/git/cmd.py
index f847166c..9bd95553 100644
--- a/git/cmd.py
+++ b/git/cmd.py
@@ -38,6 +38,9 @@ log = logging.getLogger('git.cmd')
 
 __all__ = ('Git', )
 
+if sys.platform != 'win32':
+    WindowsError = OSError
+
 
 # ==============================================================================
 ## @name Utilities
@@ -203,11 +206,18 @@ class Git(LazyMixin):
             self.args = args
 
         def __del__(self):
-            self.proc.stdout.close()
-            self.proc.stderr.close()
+            if self.proc is None:
+                return
+
+            proc = self.proc
+            self.proc = None
+            if proc.stdin:
+                proc.stdin.close()
+            proc.stdout.close()
+            proc.stderr.close()
 
             # did the process finish already so we have a return code ?
-            if self.proc.poll() is not None:
+            if proc.poll() is not None:
                 return
 
             # can be that nothing really exists anymore ...
@@ -216,8 +226,8 @@ class Git(LazyMixin):
 
             # try to kill it
             try:
-                os.kill(self.proc.pid, 2)   # interrupt signal
-                self.proc.wait()    # ensure process goes away
+                os.kill(proc.pid, 2)   # interrupt signal
+                proc.wait()    # ensure process goes away
             except (OSError, WindowsError):
                 pass  # ignore error when process already died
             except AttributeError:
@@ -225,7 +235,7 @@ class Git(LazyMixin):
                 # for some reason, providing None for stdout/stderr still prints something. This is why
                 # we simply use the shell and redirect to nul. Its slower than CreateProcess, question
                 # is whether we really want to see all these messages. Its annoying no matter what.
-                call(("TASKKILL /F /T /PID %s 2>nul 1>nul" % str(self.proc.pid)), shell=True)
+                call(("TASKKILL /F /T /PID %s 2>nul 1>nul" % str(proc.pid)), shell=True)
             # END exception handling
 
         def __getattr__(self, attr):
diff --git a/git/refs/log.py b/git/refs/log.py
index f8dc88da..fed13608 100644
--- a/git/refs/log.py
+++ b/git/refs/log.py
@@ -204,6 +204,7 @@ class RefLog(list, Serializable):
                 return
             yield new_entry(line.strip())
         # END endless loop
+        stream.close()
 
     @classmethod
     def entry_at(cls, filepath, index):
diff --git a/git/repo/base.py b/git/repo/base.py
index e8db3540..934a6f03 100644
--- a/git/repo/base.py
+++ b/git/repo/base.py
@@ -168,6 +168,10 @@ class Repo(object):
             args.append(self.git)
         self.odb = odbt(*args)
 
+    def __del__(self):
+        if self.git:
+            self.git.clear_cache()
+
     def __eq__(self, rhs):
         if isinstance(rhs, Repo):
             return self.git_dir == rhs.git_dir
diff --git a/git/test/lib/helper.py b/git/test/lib/helper.py
index bd679512..6c9f33c6 100644
--- a/git/test/lib/helper.py
+++ b/git/test/lib/helper.py
@@ -279,6 +279,11 @@ class TestBase(TestCase):
         """
         cls.rorepo = Repo(GIT_REPO)
 
+    @classmethod
+    def tearDownClass(cls):
+        cls.rorepo.git.clear_cache()
+        cls.rorepo.git = None
+
     def _make_file(self, rela_path, data, repo=None):
         """
         Create a file at the given path relative to our repository, filled
diff --git a/git/test/performance/lib.py b/git/test/performance/lib.py
index 5c138f17..46a198d4 100644
--- a/git/test/performance/lib.py
+++ b/git/test/performance/lib.py
@@ -59,6 +59,12 @@ class TestBigRepoR(TestBase):
         self.gitrorepo = Repo(repo_path, odbt=GitCmdObjectDB)
         self.puregitrorepo = Repo(repo_path, odbt=GitDB)
 
+    def tearDown(self):
+        self.gitrorepo.git.clear_cache()
+        self.gitrorepo = None
+        self.puregitrorepo.git.clear_cache()
+        self.puregitrorepo = None
+
 
 class TestBigRepoRW(TestBigRepoR):
 
@@ -78,7 +84,12 @@ class TestBigRepoRW(TestBigRepoR):
         self.puregitrwrepo = Repo(dirname, odbt=GitDB)
 
     def tearDown(self):
+        super(TestBigRepoRW, self).tearDown()
         if self.gitrwrepo is not None:
             shutil.rmtree(self.gitrwrepo.working_dir)
+            self.gitrwrepo.git.clear_cache()
+        self.gitrwrepo = None
+        self.puregitrwrepo.git.clear_cache()
+        self.puregitrwrepo = None
 
 #} END base classes
diff --git a/git/test/test_repo.py b/git/test/test_repo.py
index eb831ce3..260f7d68 100644
--- a/git/test/test_repo.py
+++ b/git/test/test_repo.py
@@ -656,3 +656,20 @@ class TestRepo(TestBase):
         open(git_file_path, 'wb').write(('gitdir: %s\n' % real_path_abs).encode('ascii'))
         git_file_repo = Repo(rwrepo.working_tree_dir)
         assert os.path.abspath(git_file_repo.git_dir) == real_path_abs
+
+    def test_file_handle_leaks(self):
+        def last_commit(repo, rev, path):
+            commit = next(repo.iter_commits(rev, path, max_count=1))
+            commit.tree[path]
+
+        # This is based on this comment
+        # https://github.com/gitpython-developers/GitPython/issues/60#issuecomment-23558741
+        # And we expect to set max handles to a low value, like 64
+        # You should set ulimit -n X, see .travis.yml
+        # The loops below would easily create 500 handles if these would leak (4 pipes + multiple mapped files)
+        for i in range(64):
+            for repo_type in (GitCmdObjectDB, GitDB):
+                repo = Repo(self.rorepo.working_tree_dir, odbt=repo_type)
+                last_commit(repo, 'master', 'git/test/test_base.py')
+            # end for each repository type
+        # end for each iteration