Merge pull request #24523 from JuliaLang/ksh/gcstrikesback

GC fixes in libgit2 round 2 electric boogaloo
JuliaLang · Nov 8, 2017 · 8d67226 · 8d67226
2 parents 3077732 + 1684a6a
commit 8d67226
Show file tree

Hide file tree

Showing 9 changed files with 93 additions and 51 deletions.
diff --git a/base/libgit2/blame.jl b/base/libgit2/blame.jl
@@ -32,10 +32,13 @@ function Base.getindex(blame::GitBlame, i::Integer)
     if !(1 <= i <= counthunks(blame))
         throw(BoundsError(blame, (i,)))
     end
-    hunk_ptr = ccall((:git_blame_get_hunk_byindex, :libgit2),
-                      Ptr{BlameHunk},
-                      (Ptr{Void}, Csize_t), blame.ptr, i-1)
-    return unsafe_load(hunk_ptr)
+    Base.@gc_preserve blame begin
+        hunk_ptr = ccall((:git_blame_get_hunk_byindex, :libgit2),
+                          Ptr{BlameHunk},
+                          (Ptr{Void}, Csize_t), blame.ptr, i-1)
+        elem = unsafe_load(hunk_ptr)
+    end
+    return elem
 end
 
 function Base.show(io::IO, blame_hunk::BlameHunk)

diff --git a/base/libgit2/index.jl b/base/libgit2/index.jl
@@ -168,11 +168,14 @@ function Base.count(idx::GitIndex)
 end
 
 function Base.getindex(idx::GitIndex, i::Integer)
-    ie_ptr = ccall((:git_index_get_byindex, :libgit2),
-                   Ptr{IndexEntry},
-                   (Ptr{Void}, Csize_t), idx.ptr, i-1)
-    ie_ptr == C_NULL && return nothing
-    return unsafe_load(ie_ptr)
+    Base.@gc_preserve idx begin
+        ie_ptr = ccall((:git_index_get_byindex, :libgit2),
+                       Ptr{IndexEntry},
+                       (Ptr{Void}, Csize_t), idx.ptr, i-1)
+        ie_ptr == C_NULL && return nothing
+        elem = unsafe_load(ie_ptr)
+    end
+    return elem
 end
 
 function Base.find(path::String, idx::GitIndex)

diff --git a/base/libgit2/merge.jl b/base/libgit2/merge.jl
@@ -44,7 +44,10 @@ function GitAnnotated(repo::GitRepo, comittish::AbstractString)
 end
 
 function GitHash(ann::GitAnnotated)
-    unsafe_load(ccall((:git_annotated_commit_id, :libgit2), Ptr{GitHash}, (Ptr{Void},), ann.ptr))
+    Base.@gc_preserve ann begin
+        oid = unsafe_load(ccall((:git_annotated_commit_id, :libgit2), Ptr{GitHash}, (Ptr{Void},), ann.ptr))
+    end
+    return oid
 end
 
 """

diff --git a/base/libgit2/oid.jl b/base/libgit2/oid.jl
@@ -106,9 +106,12 @@ Get the identifier (`GitHash`) of the object referred to by the direct reference
 function GitHash(ref::GitReference)
     isempty(ref) && return GitHash()
     reftype(ref) != Consts.REF_OID && return GitHash()
-    oid_ptr = ccall((:git_reference_target, :libgit2), Ptr{UInt8}, (Ptr{Void},), ref.ptr)
-    oid_ptr == C_NULL && return GitHash()
-    return GitHash(oid_ptr)
+    Base.@gc_preserve ref begin
+        oid_ptr = ccall((:git_reference_target, :libgit2), Ptr{UInt8}, (Ptr{Void},), ref.ptr)
+        oid_ptr == C_NULL && return GitHash()
+        oid = GitHash(oid_ptr)
+    end
+    return oid
 end
 
 

diff --git a/base/libgit2/rebase.jl b/base/libgit2/rebase.jl
@@ -32,23 +32,29 @@ function Base.getindex(rb::GitRebase, i::Integer)
     if !(1 <= i <= count(rb))
         throw(BoundsError(rb, (i,)))
     end
-    rb_op_ptr = ccall((:git_rebase_operation_byindex, :libgit2),
-                      Ptr{RebaseOperation},
-                      (Ptr{Void}, Csize_t), rb.ptr, i-1)
-    return unsafe_load(rb_op_ptr)
+    Base.@gc_preserve rb begin
+        rb_op_ptr = ccall((:git_rebase_operation_byindex, :libgit2),
+                          Ptr{RebaseOperation},
+                          (Ptr{Void}, Csize_t), rb.ptr, i-1)
+        rb_op = unsafe_load(rb_op_ptr)
+    end
+    return rb_op
 end
 
 function Base.next(rb::GitRebase)
     rb_op_ptr_ptr = Ref{Ptr{RebaseOperation}}(C_NULL)
-    try
-        @check ccall((:git_rebase_next, :libgit2), Cint,
-                      (Ptr{Ptr{RebaseOperation}}, Ptr{Void}),
-                       rb_op_ptr_ptr, rb.ptr)
-    catch err
-        err.code == Error.ITEROVER && return nothing
-        rethrow(err)
+    Base.@gc_preserve rb begin
+        try
+            @check ccall((:git_rebase_next, :libgit2), Cint,
+                          (Ptr{Ptr{RebaseOperation}}, Ptr{Void}),
+                           rb_op_ptr_ptr, rb.ptr)
+        catch err
+            err.code == Error.ITEROVER && return nothing
+            rethrow(err)
+        end
+        rb_op_ptr = unsafe_load(rb_op_ptr_ptr[])
     end
-    return unsafe_load(rb_op_ptr_ptr[])
+    return rb_op_ptr
 end
 
 function Base.show(io::IO, rb::GitRebase)

diff --git a/base/libgit2/reference.jl b/base/libgit2/reference.jl
@@ -62,9 +62,12 @@ julia> LibGit2.shortname(branch_ref)
 """
 function shortname(ref::GitReference)
     isempty(ref) && return ""
-    name_ptr = ccall((:git_reference_shorthand, :libgit2), Cstring, (Ptr{Void},), ref.ptr)
-    name_ptr == C_NULL && return ""
-    return unsafe_string(name_ptr)
+    Base.@gc_preserve ref begin
+        name_ptr = ccall((:git_reference_shorthand, :libgit2), Cstring, (Ptr{Void},), ref.ptr)
+        name_ptr == C_NULL && return ""
+        name = unsafe_string(name_ptr)
+    end
+    return name
 end
 
 """
@@ -89,9 +92,12 @@ reference, returns an empty string.
 function fullname(ref::GitReference)
     isempty(ref) && return ""
     reftype(ref) == Consts.REF_OID && return ""
-    rname = ccall((:git_reference_symbolic_target, :libgit2), Cstring, (Ptr{Void},), ref.ptr)
-    rname == C_NULL && return ""
-    return unsafe_string(rname)
+    Base.@gc_preserve ref begin
+        rname = ccall((:git_reference_symbolic_target, :libgit2), Cstring, (Ptr{Void},), ref.ptr)
+        rname == C_NULL && return ""
+        name = unsafe_string(rname)
+    end
+    return name
 end
 
 """
@@ -101,17 +107,23 @@ Return the full name of `ref`.
 """
 function name(ref::GitReference)
     isempty(ref) && return ""
-    name_ptr = ccall((:git_reference_name, :libgit2), Cstring, (Ptr{Void},), ref.ptr)
-    name_ptr == C_NULL && return ""
-    return unsafe_string(name_ptr)
+    Base.@gc_preserve ref begin
+        name_ptr = ccall((:git_reference_name, :libgit2), Cstring, (Ptr{Void},), ref.ptr)
+        name_ptr == C_NULL && return ""
+        name = unsafe_string(name_ptr)
+    end
+    return name
 end
 
 function branch(ref::GitReference)
     isempty(ref) && return ""
     str_ptr_ptr = Ref{Cstring}()
-    @check ccall((:git_branch_name, :libgit2), Cint,
-                  (Ptr{Cstring}, Ptr{Void},), str_ptr_ptr, ref.ptr)
-    return unsafe_string(str_ptr_ptr[])
+    Base.@gc_preserve ref begin
+        @check ccall((:git_branch_name, :libgit2), Cint,
+                      (Ptr{Cstring}, Ptr{Void},), str_ptr_ptr, ref.ptr)
+        str = unsafe_string(str_ptr_ptr[])
+    end
+    return str
 end
 
 function ishead(ref::GitReference)

diff --git a/base/libgit2/status.jl b/base/libgit2/status.jl
@@ -24,12 +24,15 @@ end
 
 function Base.getindex(status::GitStatus, i::Integer)
     1 <= i <= length(status) || throw(BoundsError())
-    entry_ptr = ccall((:git_status_byindex, :libgit2),
-                      Ptr{StatusEntry},
-                      (Ptr{Void}, Csize_t),
-                      status.ptr, i-1)
-    entry_ptr == C_NULL && throw(Error.GitError(Error.ERROR))
-    return unsafe_load(entry_ptr)
+    Base.@gc_preserve status begin
+        entry_ptr = ccall((:git_status_byindex, :libgit2),
+                          Ptr{StatusEntry},
+                          (Ptr{Void}, Csize_t),
+                          status.ptr, i-1)
+        entry_ptr == C_NULL && throw(Error.GitError(Error.ERROR))
+        entry = unsafe_load(entry_ptr)
+    end
+    return entry
 end
 
 """

diff --git a/base/libgit2/tag.jl b/base/libgit2/tag.jl
@@ -57,9 +57,12 @@ end
 The name of `tag` (e.g. `"v0.5"`).
 """
 function name(tag::GitTag)
-    str_ptr = ccall((:git_tag_name, :libgit2), Cstring, (Ptr{Void},), tag.ptr)
-    str_ptr == C_NULL && throw(Error.GitError(Error.ERROR))
-    return unsafe_string(str_ptr)
+    Base.@gc_preserve tag begin
+        str_ptr = ccall((:git_tag_name, :libgit2), Cstring, (Ptr{Void},), tag.ptr)
+        str_ptr == C_NULL && throw(Error.GitError(Error.ERROR))
+        str = unsafe_string(str_ptr)
+    end
+    return str
 end
 
 # should we return the actual object? i.e. git_tag_target?
@@ -69,9 +72,12 @@ end
 The `GitHash` of the target object of `tag`.
 """
 function target(tag::GitTag)
-    oid_ptr = ccall((:git_tag_target_id, :libgit2), Ptr{GitHash}, (Ptr{Void},), tag.ptr)
-    oid_ptr == C_NULL && throw(Error.GitError(Error.ERROR))
-    return unsafe_load(oid_ptr)
+    Base.@gc_preserve tag begin
+        oid_ptr = ccall((:git_tag_target_id, :libgit2), Ptr{GitHash}, (Ptr{Void},), tag.ptr)
+        oid_ptr == C_NULL && throw(Error.GitError(Error.ERROR))
+        str = unsafe_load(oid_ptr)
+    end
+    return str
 end
 
 Base.show(io::IO, tag::GitTag) = print(io, "GitTag:\nTag name: $(name(tag)) target: $(target(tag))")
diff --git a/base/libgit2/tree.jl b/base/libgit2/tree.jl
@@ -66,8 +66,11 @@ end
 Return the [`GitHash`](@ref) of the object to which `te` refers.
 """
 function entryid(te::GitTreeEntry)
-    oid_ptr = ccall((:git_tree_entry_id, :libgit2), Ptr{UInt8}, (Ptr{Void},), te.ptr)
-    return GitHash(oid_ptr)
+    Base.@gc_preserve te begin
+        oid_ptr = ccall((:git_tree_entry_id, :libgit2), Ptr{UInt8}, (Ptr{Void},), te.ptr)
+        oid = GitHash(oid_ptr)
+    end
+    return oid
 end
 
 function Base.count(tree::GitTree)