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>

7 files changed, 43 insertions, 37 deletions

diff --git a/src/blob.c b/src/blob.c
index f271cc7f6..a9e765b40 100644
--- 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 dff3f1e34..974999a4a 100644
--- 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 de02ef5f2..7891893a0 100644
--- 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 d3852d3ad..d6909ae4f 100644
--- 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 4f4a4a863..5318ed45c 100644
--- 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 872cdbc43..a1cd0ebb7 100644
--- 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 01cc0dc8f..1ac2067c2 100644
--- 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;