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.

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

Add reference count to LibGit2 objects #20124

Merged
merged 3 commits into from
Jan 21, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 8 additions & 1 deletion base/libgit2/libgit2.jl
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ export with, GitRepo, GitConfig
const GITHUB_REGEX =
r"^(?:git@|git://|https://(?:[\w\.\+\-]+@)?)github.com[:/](([^/].+)/(.+?))(?:\.git)?$"i

const REFCOUNT = Threads.Atomic{UInt}()

include("utils.jl")
include("consts.jl")
include("types.jl")
Expand Down Expand Up @@ -603,8 +605,13 @@ function __init__()

err = ccall((:git_libgit2_init, :libgit2), Cint, ())
err > 0 || throw(ErrorException("error initializing LibGit2 module"))
REFCOUNT[] = 1

atexit() do
ccall((:git_libgit2_shutdown, :libgit2), Cint, ())
if Threads.atomic_sub!(REFCOUNT, UInt(1)) == 1
# refcount zero, no objects to be finalized
ccall((:git_libgit2_shutdown, :libgit2), Cint, ())
end
end

@static if is_linux()
Expand Down
10 changes: 10 additions & 0 deletions base/libgit2/types.jl
Original file line number Diff line number Diff line change
Expand Up @@ -449,9 +449,12 @@ for (typ, reporef, sup, cname) in [
@eval type $typ <: $sup
ptr::Ptr{Void}
function $typ(ptr::Ptr{Void},fin=true)
# fin=false should only be used when the pointer should not be free'd
# e.g. from within callback functions which are passed a pointer
@assert ptr != C_NULL
obj = new(ptr)
if fin
Threads.atomic_add!(REFCOUNT, UInt(1))
finalizer(obj, Base.close)
end
return obj
Expand All @@ -464,12 +467,14 @@ for (typ, reporef, sup, cname) in [
function $typ(repo::GitRepo, ptr::Ptr{Void})
@assert ptr != C_NULL
obj = new(Nullable(repo), ptr)
Threads.atomic_add!(REFCOUNT, UInt(1))
finalizer(obj, Base.close)
return obj
end
function $typ(ptr::Ptr{Void})
@assert ptr != C_NULL
obj = new(Nullable{GitRepo}(), ptr)
Threads.atomic_add!(REFCOUNT, UInt(1))
finalizer(obj, Base.close)
return obj
end
Expand All @@ -481,6 +486,7 @@ for (typ, reporef, sup, cname) in [
function $typ(repo::GitRepo, ptr::Ptr{Void})
@assert ptr != C_NULL
obj = new(repo, ptr)
Threads.atomic_add!(REFCOUNT, UInt(1))
finalizer(obj, Base.close)
return obj
end
Expand All @@ -490,6 +496,10 @@ for (typ, reporef, sup, cname) in [
if obj.ptr != C_NULL
ccall(($(string(cname, :_free)), :libgit2), Void, (Ptr{Void},), obj.ptr)
obj.ptr = C_NULL
if Threads.atomic_sub!(REFCOUNT, UInt(1)) == 1
# will the last finalizer please turn out the lights?
ccall((:git_libgit2_shutdown, :libgit2), Cint, ())
Copy link
Member

Choose a reason for hiding this comment

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

What if the number of libgit2 objects drops to 0 at runtime. Is that ok?

Copy link
Contributor

Choose a reason for hiding this comment

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

The __init__ holds a global reference so the reference should not drop to 0.

end
end
end
end
Expand Down