From 9b8d1093e0ed518b7b2c5391dcc11f4ee211b306 Mon Sep 17 00:00:00 2001 From: Simon Byrne Date: Thu, 19 Jan 2017 09:52:29 +0000 Subject: [PATCH 1/3] Add reference count to LibGit2 objects Fixes segfault from calling `git_libgit2_shutdown` before finalizers (#20109). --- base/libgit2/libgit2.jl | 10 +++++++++- base/libgit2/types.jl | 9 +++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/base/libgit2/libgit2.jl b/base/libgit2/libgit2.jl index e6f4c97a24193..9965b66d79c1f 100644 --- a/base/libgit2/libgit2.jl +++ b/base/libgit2/libgit2.jl @@ -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)) + include("utils.jl") include("consts.jl") include("types.jl") @@ -603,8 +605,14 @@ 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, ()) + REFCOUNT[] -= 1 + if REFCOUNT[] == 0 + # no objects to be finalized + ccall((:git_libgit2_shutdown, :libgit2), Cint, ()) + end end @static if is_linux() diff --git a/base/libgit2/types.jl b/base/libgit2/types.jl index b9b86811883c2..3bbef28932e49 100644 --- a/base/libgit2/types.jl +++ b/base/libgit2/types.jl @@ -452,6 +452,7 @@ for (typ, reporef, sup, cname) in [ @assert ptr != C_NULL obj = new(ptr) if fin + REFCOUNT[] += 1 finalizer(obj, Base.close) end return obj @@ -464,12 +465,14 @@ for (typ, reporef, sup, cname) in [ function $typ(repo::GitRepo, ptr::Ptr{Void}) @assert ptr != C_NULL obj = new(Nullable(repo), ptr) + REFCOUNT[] += 1 finalizer(obj, Base.close) return obj end function $typ(ptr::Ptr{Void}) @assert ptr != C_NULL obj = new(Nullable{GitRepo}(), ptr) + REFCOUNT[] += 1 finalizer(obj, Base.close) return obj end @@ -481,6 +484,7 @@ for (typ, reporef, sup, cname) in [ function $typ(repo::GitRepo, ptr::Ptr{Void}) @assert ptr != C_NULL obj = new(repo, ptr) + REFCOUNT[] += 1 finalizer(obj, Base.close) return obj end @@ -490,6 +494,11 @@ 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 + REFCOUNT[] -= 1 + if REFCOUNT[] == 0 + # will the last finalizer please turn out the lights? + ccall((:git_libgit2_shutdown, :libgit2), Cint, ()) + end end end end From 4fa8a9a5dbb82f7ca8c3d9e3f8b9720ab729bc90 Mon Sep 17 00:00:00 2001 From: Simon Byrne Date: Fri, 20 Jan 2017 09:30:19 +0000 Subject: [PATCH 2/3] set initial refcount to 1 --- base/libgit2/libgit2.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/libgit2/libgit2.jl b/base/libgit2/libgit2.jl index 9965b66d79c1f..2cd266ebd2dfd 100644 --- a/base/libgit2/libgit2.jl +++ b/base/libgit2/libgit2.jl @@ -605,7 +605,7 @@ function __init__() err = ccall((:git_libgit2_init, :libgit2), Cint, ()) err > 0 || throw(ErrorException("error initializing LibGit2 module")) - REFCOUNT[] += 1 + REFCOUNT[] = 1 atexit() do REFCOUNT[] -= 1 From 20e24bcad0e56ed6f7c114eab6f75c7732c624ca Mon Sep 17 00:00:00 2001 From: Simon Byrne Date: Fri, 20 Jan 2017 09:46:57 +0000 Subject: [PATCH 3/3] Use atomic counters for REFCOUNT --- base/libgit2/libgit2.jl | 7 +++---- base/libgit2/types.jl | 13 +++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/base/libgit2/libgit2.jl b/base/libgit2/libgit2.jl index 2cd266ebd2dfd..a00858dd3e8d1 100644 --- a/base/libgit2/libgit2.jl +++ b/base/libgit2/libgit2.jl @@ -9,7 +9,7 @@ export with, GitRepo, GitConfig const GITHUB_REGEX = r"^(?:git@|git://|https://(?:[\w\.\+\-]+@)?)github.com[:/](([^/].+)/(.+?))(?:\.git)?$"i -const REFCOUNT = Ref(zero(UInt)) +const REFCOUNT = Threads.Atomic{UInt}() include("utils.jl") include("consts.jl") @@ -608,9 +608,8 @@ function __init__() REFCOUNT[] = 1 atexit() do - REFCOUNT[] -= 1 - if REFCOUNT[] == 0 - # no objects to be finalized + if Threads.atomic_sub!(REFCOUNT, UInt(1)) == 1 + # refcount zero, no objects to be finalized ccall((:git_libgit2_shutdown, :libgit2), Cint, ()) end end diff --git a/base/libgit2/types.jl b/base/libgit2/types.jl index 3bbef28932e49..6a3e70c9f1751 100644 --- a/base/libgit2/types.jl +++ b/base/libgit2/types.jl @@ -449,10 +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 - REFCOUNT[] += 1 + Threads.atomic_add!(REFCOUNT, UInt(1)) finalizer(obj, Base.close) end return obj @@ -465,14 +467,14 @@ for (typ, reporef, sup, cname) in [ function $typ(repo::GitRepo, ptr::Ptr{Void}) @assert ptr != C_NULL obj = new(Nullable(repo), ptr) - REFCOUNT[] += 1 + 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) - REFCOUNT[] += 1 + Threads.atomic_add!(REFCOUNT, UInt(1)) finalizer(obj, Base.close) return obj end @@ -484,7 +486,7 @@ for (typ, reporef, sup, cname) in [ function $typ(repo::GitRepo, ptr::Ptr{Void}) @assert ptr != C_NULL obj = new(repo, ptr) - REFCOUNT[] += 1 + Threads.atomic_add!(REFCOUNT, UInt(1)) finalizer(obj, Base.close) return obj end @@ -494,8 +496,7 @@ 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 - REFCOUNT[] -= 1 - if REFCOUNT[] == 0 + if Threads.atomic_sub!(REFCOUNT, UInt(1)) == 1 # will the last finalizer please turn out the lights? ccall((:git_libgit2_shutdown, :libgit2), Cint, ()) end