Fix segmentation fault when freeing a repository

Disable garbage collection of cross-references to prevent
double-freeing. Internal reference management is now done
with a separate method.

Signed-off-by: Vicent Marti <tanoku@gmail.com>

diff --git a/src/blob.c b/src/blob.c
index f271cc7f657ab20608b7262fd0341f522e4776a2..a9e765b407306b19dc4a2259199b36edb7083f82 100644 (file)
--- a/src/blob.c
+++ b/src/blob.c
@@ -132,7 +132,7 @@ int git_blob_writefile(git_oid *written_id, git_repository *repo, const char *pa
 
        /* FIXME: maybe we don't want to free this already?
         * the user may want to access it again */
-       git_object_close((git_object *)blob);
+       GIT_OBJECT_DECREF(repo, blob);
        return GIT_SUCCESS;
 }
 

diff --git a/src/commit.c b/src/commit.c
index dff3f1e349b3dfb304851f2057d254d7932307f7..974999a4a961345f7d652defcfde7b05f6bca252 100644 (file)
--- a/src/commit.c
+++ b/src/commit.c
@@ -48,7 +48,7 @@ static void clear_parents(git_commit *commit)
 
        for (i = 0; i < commit->parents.length; ++i) {
                git_commit *parent = git_vector_get(&commit->parents, i);
-               git_object_close((git_object *)parent);
+               GIT_OBJECT_DECREF(commit->object.repo, parent);
        }
 
        git_vector_clear(&commit->parents);
@@ -62,7 +62,7 @@ void git_commit__free(git_commit *commit)
        git_signature_free(commit->author);
        git_signature_free(commit->committer);
 
-       git_object_close((git_object *)commit->tree);
+       GIT_OBJECT_DECREF(commit->object.repo, commit->tree);
 
        free(commit->message);
        free(commit->message_short);
@@ -130,7 +130,7 @@ int commit_parse_buffer(git_commit *commit, void *data, size_t len, unsigned int
        if ((error = git__parse_oid(&oid, &buffer, buffer_end, "tree ")) < GIT_SUCCESS)
                return error;
 
-       git_object_close((git_object *)commit->tree);
+       GIT_OBJECT_DECREF(commit->object.repo, commit->tree);
        if ((error = git_object_lookup((git_object **)&commit->tree, commit->object.repo, &oid, GIT_OBJ_TREE)) < GIT_SUCCESS)
                return error;
 
@@ -249,7 +249,7 @@ const git_tree *git_commit_tree(git_commit *commit)
        if (!commit->object.in_memory && commit->tree == NULL)
                git_commit__parse_full(commit);
 
-       GIT_OBJECT_INCREF(commit->tree);
+       GIT_OBJECT_INCREF(commit->object.repo, commit->tree);
        return commit->tree;
 }
 
@@ -283,7 +283,7 @@ git_commit *git_commit_parent(git_commit *commit, unsigned int n)
        assert(commit);
 
        parent = git_vector_get(&commit->parents, n);
-       GIT_OBJECT_INCREF(parent);
+       GIT_OBJECT_INCREF(commit->object.repo, parent);
        return parent;
 }
 
@@ -293,8 +293,8 @@ void git_commit_set_tree(git_commit *commit, git_tree *tree)
        commit->object.modified = 1;
        CHECK_FULL_PARSE();
 
-       git_object_close((git_object *)commit->tree);
-       GIT_OBJECT_INCREF(tree);
+       GIT_OBJECT_DECREF(commit->object.repo, commit->tree);
+       GIT_OBJECT_INCREF(commit->object.repo, tree);
        commit->tree = tree;
 }
 
@@ -353,6 +353,6 @@ int git_commit_add_parent(git_commit *commit, git_commit *new_parent)
 
        CHECK_FULL_PARSE();
        commit->object.modified = 1;
-       GIT_OBJECT_INCREF(new_parent);
+       GIT_OBJECT_INCREF(commit->object.repo, new_parent);
        return git_vector_insert(&commit->parents, new_parent);
 }

diff --git a/src/object.c b/src/object.c
index de02ef5f252a4b330de4b2dc5add5b69d1b6f755..7891893a005321a5913876beddb84d8acc3fe6ea 100644 (file)
--- a/src/object.c
+++ b/src/object.c
@@ -277,7 +277,7 @@ int git_object_lookup(git_object **object_out, git_repository *repo, const git_o
        object = git_hashtable_lookup(repo->objects, id);
        if (object != NULL) {
                *object_out = object;
-               GIT_OBJECT_INCREF(object);
+               GIT_OBJECT_INCREF(repo, object);
                return GIT_SUCCESS;
        }
 
@@ -330,7 +330,7 @@ int git_object_lookup(git_object **object_out, git_repository *repo, const git_o
        git_object__source_close(object);
        git_hashtable_insert(repo->objects, &object->id, object);
 
-       GIT_OBJECT_INCREF(object);
+       GIT_OBJECT_INCREF(repo, object);
        *object_out = object;
        return GIT_SUCCESS;
 }
@@ -384,15 +384,6 @@ void git_object__free(git_object *object)
 
        git_object__source_close(object);
 
-       if (object->repo != NULL) {
-               if (object->in_memory) {
-                       int idx = git_vector_search(&object->repo->memory_objects, object);
-                       git_vector_remove(&object->repo->memory_objects, idx);
-               } else {
-                       git_hashtable_remove(object->repo->objects, &object->id);
-               }
-       }
-
        switch (object->source.raw.type) {
        case GIT_OBJ_COMMIT:
                git_commit__free((git_commit *)object);
@@ -421,8 +412,18 @@ void git_object_close(git_object *object)
        if (object == NULL)
                return;
 
-       if (--object->refcount <= 0)
+       if (--object->refcount <= 0) {
+               if (object->repo != NULL) {
+                       if (object->in_memory) {
+                               int idx = git_vector_search(&object->repo->memory_objects, object);
+                               git_vector_remove(&object->repo->memory_objects, idx);
+                       } else {
+                               git_hashtable_remove(object->repo->objects, &object->id);
+                       }
+               }
+
                git_object__free(object);
+       }
 }
 
 const git_oid *git_object_id(const git_object *obj)

diff --git a/src/repository.c b/src/repository.c
index d3852d3adca7a16cd0e52ca06f45198d77616cf8..d6909ae4f7faf2a3ff92dfafb85f057693dcd725 100644 (file)
--- a/src/repository.c
+++ b/src/repository.c
@@ -216,6 +216,7 @@ static git_repository *repository_alloc()
                return NULL;
        }
 
+       repo->gc_enabled = 1;
        return repo;
 }
 
@@ -387,11 +388,7 @@ void git_repository_free(git_repository *repo)
        if (repo == NULL)
                return;
 
-       /* Increment the refcount of all the objects in the repository
-        * to prevent freeing dependencies */
-       GIT_HASHTABLE_FOREACH(repo->objects, _unused, object,
-               GIT_OBJECT_INCREF(object);
-       );
+       repo->gc_enabled = 0;
 
        /* force free all the objects */
        GIT_HASHTABLE_FOREACH(repo->objects, _unused, object,

diff --git a/src/repository.h b/src/repository.h
index 4f4a4a863f44493556b922a7780bbe79aa8768dd..5318ed45cd7df12561a096ff5f4f69bd5cf4a195 100644 (file)
--- a/src/repository.h
+++ b/src/repository.h
@@ -5,6 +5,7 @@
 #include "git2/oid.h"
 #include "git2/odb.h"
 #include "git2/repository.h"
+#include "git2/object.h"
 
 #include "hashtable.h"
 #include "index.h"
@@ -44,7 +45,7 @@ struct git_repository {
        char *path_odb;
        char *path_workdir;
 
-       unsigned is_bare:1;
+       unsigned is_bare:1, gc_enabled:1;
 };
 
 int git_object__source_open(git_object *object);
@@ -60,12 +61,19 @@ int git__source_write(git_odb_source *source, const void *bytes, size_t len);
 int git__parse_oid(git_oid *oid, char **buffer_out, const char *buffer_end, const char *header);
 int git__write_oid(git_odb_source *src, const char *header, const git_oid *oid);
 
-#define GIT_OBJECT_INCREF(ob) git_object__incref((git_object *)(ob))
+#define GIT_OBJECT_INCREF(repo, ob) git_object__incref((repo), (git_object *)(ob))
+#define GIT_OBJECT_DECREF(repo, ob) git_object__decref((repo), (git_object *)(ob)) 
 
-GIT_INLINE(void) git_object__incref(struct git_object *object)
+GIT_INLINE(void) git_object__incref(git_repository *repo, struct git_object *object)
 {
-       if (object)
+       if (repo && repo->gc_enabled && object)
                object->refcount++;
 }
 
+GIT_INLINE(void) git_object__decref(git_repository *repo, struct git_object *object)
+{
+       if (repo && repo->gc_enabled && object)
+               git_object_close(object);
+}
+
 #endif

diff --git a/src/revwalk.c b/src/revwalk.c
index 872cdbc43dae7a3822a0076c6d5769d5d820ad89..a1cd0ebb7e7ad06e7bb996dc87ac64b680431c51 100644 (file)
--- a/src/revwalk.c
+++ b/src/revwalk.c
@@ -114,7 +114,7 @@ static git_revwalk_commit *commit_to_walkcommit(git_revwalk *walk, git_commit *c
        memset(commit, 0x0, sizeof(git_revwalk_commit));
 
        commit->commit_object = commit_object;
-       GIT_OBJECT_INCREF(commit_object);
+       GIT_OBJECT_INCREF(walk->repo, commit_object);
 
        git_hashtable_insert(walk->commits, commit_object, commit);
 
@@ -230,7 +230,7 @@ int git_revwalk_next(git_commit **commit, git_revwalk *walk)
        while ((next = walk->next(&walk->iterator)) != NULL) {
                if (!next->uninteresting) {
                        *commit = next->commit_object;
-                       GIT_OBJECT_INCREF(*commit);
+                       GIT_OBJECT_INCREF(walk->repo, *commit);
                        return GIT_SUCCESS;
                }
        }
@@ -248,7 +248,7 @@ void git_revwalk_reset(git_revwalk *walk)
        assert(walk);
 
        GIT_HASHTABLE_FOREACH(walk->commits, _unused, commit, {
-               git_object_close((git_object *)commit->commit_object);
+               GIT_OBJECT_DECREF(walk->repo, commit->commit_object);
                git_revwalk_list_clear(&commit->parents);
                free(commit);
        });

diff --git a/src/tag.c b/src/tag.c
index 01cc0dc8fedbf9b643f95b9516949e511fb59bef..1ac2067c271bb75eb09e6b9738961b2eb8c3f568 100644 (file)
--- a/src/tag.c
+++ b/src/tag.c
@@ -35,7 +35,7 @@
 void git_tag__free(git_tag *tag)
 {
        git_signature_free(tag->tagger);
-       git_object_close(tag->target);
+       GIT_OBJECT_DECREF(tag->object.repo, tag->target);
        free(tag->message);
        free(tag->tag_name);
        free(tag);
@@ -49,7 +49,7 @@ const git_oid *git_tag_id(git_tag *c)
 const git_object *git_tag_target(git_tag *t)
 {
        assert(t);
-       GIT_OBJECT_INCREF(t->target);
+       GIT_OBJECT_INCREF(t->object.repo, t->target);
        return t->target;
 }
 
@@ -57,8 +57,8 @@ void git_tag_set_target(git_tag *tag, git_object *target)
 {
        assert(tag && target);
 
-       git_object_close(tag->target);
-       GIT_OBJECT_INCREF(target);
+       GIT_OBJECT_DECREF(tag->object.repo, tag->target);
+       GIT_OBJECT_INCREF(tag->object.repo, target);
 
        tag->object.modified = 1;
        tag->target = target;