Add reference count to LibGit2 objects #20124
Conversation
Fixes segfault from calling `git_libgit2_shutdown` before finalizers (#20109).
|
I just realised that one alternative to this would be to call |
|
Can this be reviewed/merged? |
|
do we want the alternative, or this? pros/cons? |
| REFCOUNT[] -= 1 | ||
| if REFCOUNT[] == 0 | ||
| # will the last finalizer please turn out the lights? | ||
| ccall((:git_libgit2_shutdown, :libgit2), Cint, ()) |
There was a problem hiding this comment.
What if the number of libgit2 objects drops to 0 at runtime. Is that ok?
There was a problem hiding this comment.
The __init__ holds a global reference so the reference should not drop to 0.
| @@ -9,6 +9,8 @@ export with, GitRepo, GitConfig | |||
| const GITHUB_REGEX = | |||
| r"^(?:git@|git://|https://(?:[\w\.\+\-]+@)?)github.com[:/](([^/].+)/(.+?))(?:\.git)?$"i | |||
|
|
|||
| const REFCOUNT = Ref(zero(UInt)) | |||
There was a problem hiding this comment.
I would set this to 0 in __init__ also.
| @@ -603,8 +605,14 @@ function __init__() | |||
|
|
|||
| err = ccall((:git_libgit2_init, :libgit2), Cint, ()) | |||
| err > 0 || throw(ErrorException("error initializing LibGit2 module")) | |||
| REFCOUNT[] += 1 | |||
There was a problem hiding this comment.
Ideally we'll need to replace these with atomic counters but this should be good enough for now.
There was a problem hiding this comment.
I've changed these to be atomic counters: can you have a quick look over it as I'm not really familiar with them.
The only differences I can tell:
|
simonbyrne
force-pushed
the
sb/libgit2/refcount
branch
from
e50a4e2 to
20e24bc
Compare
|
Did the changes @yuyichao want get made? Can we merge this? |
Fixes segfault from calling
git_libgit2_shutdownbefore finalizers (#20109).