Remote refcount fix

pygit2/decl.h

@@ -12,6 +12,7 @@ typedef ... git_index_conflict_iterator;
 
 #define GIT_OID_RAWSZ ...
 #define GIT_PATH_MAX ...
+#define GIT_REMOTE_CALLBACKS_VERSION ...
 
 typedef struct git_oid {
 	unsigned char id[20];
@@ -136,6 +137,7 @@ int git_remote_add_push(git_remote *remote, const char *refspec);
 int git_remote_add_fetch(git_remote *remote, const char *refspec);
 int git_remote_save(const git_remote *remote);
 int git_remote_set_callbacks(git_remote *remote, const git_remote_callbacks *callbacks);
+int git_remote_init_callbacks(git_remote_callbacks *opts, unsigned int version);
 size_t git_remote_refspec_count(git_remote *remote);
 const git_refspec * git_remote_get_refspec(git_remote *remote, size_t n);
 

pygit2/remote.py

@@ -127,20 +127,6 @@ def __init__(self, repo, ptr):
         self._remote = ptr
         self._stored_exception = None
 
-        # Build the callback structure
-        callbacks = ffi.new('git_remote_callbacks *')
-        callbacks.version = 1
-        callbacks.sideband_progress = self._sideband_progress_cb
-        callbacks.transfer_progress = self._transfer_progress_cb
-        callbacks.update_tips = self._update_tips_cb
-        callbacks.credentials = self._credentials_cb
-        # We need to make sure that this handle stays alive
-        self._self_handle = ffi.new_handle(self)
-        callbacks.payload = self._self_handle
-
-        err = C.git_remote_set_callbacks(self._remote, callbacks)
-        check_error(err)
-
     def __del__(self):
         C.git_remote_free(self._remote)
 
@@ -205,17 +191,39 @@ def fetch(self, signature=None, message=None):
         Perform a fetch against this remote.
         """
 
+        defaultcallbacks = ffi.new('git_remote_callbacks *')
+        err = C.git_remote_init_callbacks(defaultcallbacks,
+                                          C.GIT_REMOTE_CALLBACKS_VERSION)
+
+        # Build the callback structure
+        callbacks = ffi.new('git_remote_callbacks *')
+        callbacks.version = 1
+        callbacks.sideband_progress = self._sideband_progress_cb
+        callbacks.transfer_progress = self._transfer_progress_cb
+        callbacks.update_tips = self._update_tips_cb
+        callbacks.credentials = self._credentials_cb
+        callbacks.payload = ffi.new_handle(self)
+
+        err = C.git_remote_set_callbacks(self._remote, callbacks)
+        check_error(err)
+
         if signature:
             ptr = signature._pointer[:]
         else:
             ptr = ffi.NULL
 
         self._stored_exception = None
-        err = C.git_remote_fetch(self._remote, ptr, to_bytes(message))
-        if self._stored_exception:
-            raise self._stored_exception
 
-        check_error(err)
+        try:
+            err = C.git_remote_fetch(self._remote, ptr, to_bytes(message))
+            if self._stored_exception:
+                raise self._stored_exception
+
+            check_error(err)
+        finally:
+            # Even on error, clear stored callbacks and reset to default
+            err = C.git_remote_set_callbacks(self._remote, defaultcallbacks)
+            check_error(err)
 
         return TransferProgress(C.git_remote_stats(self._remote))
 

test/test_commit.py

@@ -30,6 +30,7 @@
 from __future__ import absolute_import
 from __future__ import unicode_literals
 import unittest
+import sys
 
 from pygit2 import GIT_OBJ_COMMIT, Signature, Oid
 from . import utils
@@ -46,6 +47,15 @@
 
 class CommitTest(utils.BareRepoTestCase):
 
+    def test_commit_refcount(self):
+        commit = self.repo[COMMIT_SHA]
+        start = sys.getrefcount(commit)
+        tree = commit.tree
+        del tree
+        end = sys.getrefcount(commit)
+        self.assertEqual(start, end)
+
+
     def test_read_commit(self):
         commit = self.repo[COMMIT_SHA]
         self.assertEqual(COMMIT_SHA, str(commit.id))

test/test_remote.py

@@ -30,6 +30,7 @@
 
 import unittest
 import pygit2
+import sys
 from pygit2 import Oid
 from . import utils
 
@@ -163,6 +164,14 @@ def test_remote_save(self):
         self.assertEqual('http://example.com/test.git',
                          self.repo.remotes[0].url)
 
+    def test_remote_refcount(self):
+        start = sys.getrefcount(self.repo)
+        remote = self.repo.remotes[0]
+        del remote
+        end = sys.getrefcount(self.repo)
+
+        self.assertEqual(start, end)
+
     def test_add_refspec(self):
         remote = self.repo.create_remote('test_add_refspec', REMOTE_URL)
         remote.add_push('refs/heads/*:refs/heads/test_refspec/*')

test/test_repository.py

@@ -37,6 +37,7 @@
 import tempfile
 import os
 from os.path import join, realpath
+import sys
 
 # Import from pygit2
 from pygit2 import GIT_OBJ_ANY, GIT_OBJ_BLOB, GIT_OBJ_COMMIT
@@ -150,6 +151,14 @@ def test_lookup_commit_prefix(self):
             commit.message)
         self.assertRaises(ValueError, self.repo.__getitem__, too_short_prefix)
 
+    def test_lookup_commit_refcount(self):
+        start = sys.getrefcount(self.repo)
+        commit_sha = '5fe808e8953c12735680c257f56600cb0de44b10'
+        commit = self.repo[commit_sha]
+        del commit
+        end = sys.getrefcount(self.repo)
+        self.assertEqual(start, end)
+
     def test_get_path(self):
         directory = realpath(self.repo.path)
         expected = realpath(self.repo_path)