Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remote refcount fix #404

Merged
merged 6 commits into from
Aug 26, 2014
Merged

Remote refcount fix #404

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
794dbe7
set callbacks just before fetch, clear them after (fixes #403).
mduggan Aug 18, 2014
9ce6a26
Add some tests for refcounts to check for leaks
mduggan Aug 18, 2014
99433ca
Hard-code callback version because we'll need to change the defs if i…
mduggan Aug 18, 2014
bedd9ee
Don't test refcounts in pypy as they don't make sense there
mduggan Aug 18, 2014
0813ec7
Fix use of skipIf.
mduggan Aug 18, 2014
2f2d400
Ensure self handle stays alive - keeping it in callbacks is not enough.
mduggan Aug 25, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
set callbacks just before fetch, clear them after (fixes #403).
  • Loading branch information
@mduggan
mduggan committed Aug 18, 2014
commit 794dbe7b9cb5475e6c98d5e51abe6b10a7c7151c
2 changes: 2 additions & 0 deletions pygit2/decl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -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);

Expand Down
44 changes: 26 additions & 18 deletions pygit2/remote.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no need for this line. You're already setting the version above with the init function. If the version does increase, this would cause libgit2 to consider the struct to have a different layout.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I was doing it in a little bit of a weird order.. I wanted to create the "default" callbacks first so that any error could raise before the custom callbacks had been set.. but fair point about the version number being set differently on the two lines.. if it changes in one place it will need to change in both,

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))

Expand Down