repo: catch exceptions when iterating through status

When we call the block, an exception or even a call to `break` would cause us to
unwind the stack and skip any resource freeing that we may have. The solution is
to call `rb_protect` which will let us catch the exception and raise whenever we
need to.

Doing this as part of a `git_status_foreach` is unnecessarily awkward so move to
iterating over the status list ourselves and free the entry list as soon as we
notice there was an exception.

For example, this program will consume memory
indefinitely:

    require 'rugged'

    repo = Rugged::Repository.new ARGV[0]
    loop do
      repo.status do |file, status_data|
        break
      end
    end

Thanks to Aaron Patterson for discovering this and the original hacky fix.

Author: carlosmn

ext/rugged/rugged_repo.c

@@ -1502,15 +1502,6 @@ static VALUE flags_to_rb(unsigned int flags)
 	return rb_flags;
 }
 
-static int rugged__status_cb(const char *path, unsigned int flags, void *payload)
-{
-	rb_funcall((VALUE)payload, rb_intern("call"), 2,
-		rb_str_new_utf8(path), flags_to_rb(flags)
-	);
-
-	return GIT_OK;
-}
-
 static VALUE rb_git_repo_file_status(VALUE self, VALUE rb_path)
 {
 	unsigned int flags;
@@ -1527,8 +1518,10 @@ static VALUE rb_git_repo_file_status(VALUE self, VALUE rb_path)
 
 static VALUE rb_git_repo_file_each_status(VALUE self)
 {
-	int error;
+	int error, exception;
+	size_t i, nentries;
 	git_repository *repo;
+	git_status_list *list;
 
 	Data_Get_Struct(self, git_repository, repo);
 
@@ -1537,13 +1530,30 @@ static VALUE rb_git_repo_file_each_status(VALUE self)
 			"A block was expected for iterating through "
 			"the repository contents.");
 
-	error = git_status_foreach(
-		repo,
-		&rugged__status_cb,
-		(void *)rb_block_proc()
-	);
-
+	error = git_status_list_new(&list, repo, NULL);
 	rugged_exception_check(error);
+
+	nentries = git_status_list_entrycount(list);
+	for (i = 0; i < nentries; i++) {
+		const git_status_entry *entry;
+		const char *path;
+		VALUE args;
+
+		entry = git_status_byindex(list, i);
+
+		path = entry->head_to_index ?
+		       entry->head_to_index->old_file.path :
+		       entry->index_to_workdir->old_file.path;
+		args = rb_ary_new3(2, rb_str_new_utf8(path), flags_to_rb(entry->status));
+		rb_protect(rb_yield, args, &exception);
+		if (exception != 0)
+			break;
+	}
+	git_status_list_free(list);
+
+	if (exception != 0)
+		rb_jump_tag(exception);
+
 	return Qnil;
 }
 

test/status_test.rb

@@ -33,6 +33,26 @@ def setup
     @repo = FixtureRepo.from_libgit2 "status"
   end
 
+  class TestException < RuntimeError
+  end
+
+  def test_status_block_raises
+    assert_raises(TestException) do
+      @repo.status do |file, status|
+        raise TestException, "wow"
+      end
+    end
+  end
+
+  def test_status_block_breaks
+    yielded = 0
+    @repo.status do |file, status|
+      yielded += 1
+      break
+    end
+    assert_equal 1, yielded
+  end
+
   def test_status_with_callback
     actual_statuses = {}
     @repo.status do |file, status|