From a1c44aa87ada7ea6835ac90de9edd4e2cd028bb5 Mon Sep 17 00:00:00 2001
From: Benoit Daloze <eregontp@gmail.com>
Date: Thu, 31 Mar 2016 18:26:00 +0200
Subject: [PATCH] * ext/coverage/coverage.c: Fully reset coverage to not
 persist global state.   It was returning old file coverages as empty arrays
 to the user.   [ruby-core:74596] [Bug #12220] * ext/coverage/coverage.c
 (rb_coverages): remove unused static state. * thread.c: Moved and renamed
 coverage_clear_result_i to reset_coverage_i. *
 test/coverage/test_coverage.rb: improve precision of tests.

---
 ChangeLog                      | 12 ++++++++++++
 ext/coverage/coverage.c        | 25 +++++--------------------
 test/coverage/test_coverage.rb | 13 ++++++-------
 thread.c                       | 11 +++++++++++
 4 files changed, 34 insertions(+), 27 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 0fca5a1..dc03a2a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+Fri Apr  1 01:26:00 2016  Benoit Daloze  <eregontp@gmail.com>
+
+	* ext/coverage/coverage.c: Fully reset coverage to not persist global state.
+	  It was returning old file coverages as empty arrays to the user.
+	  [ruby-core:74596] [Bug #12220]
+
+	* ext/coverage/coverage.c (rb_coverages): remove unused static state.
+
+	* thread.c: Moved and renamed coverage_clear_result_i to reset_coverage_i.
+
+	* test/coverage/test_coverage.rb: improve precision of tests.
+
 Fri Apr  1 01:13:55 2016  Benoit Daloze  <eregontp@gmail.com>
 
 	* thread.c (update_coverage): Do not track coverage in loaded files
diff --git a/ext/coverage/coverage.c b/ext/coverage/coverage.c
index f621280..9a9011a 100644
--- a/ext/coverage/coverage.c
+++ b/ext/coverage/coverage.c
@@ -11,8 +11,6 @@
 #include "ruby.h"
 #include "vm_core.h"
 
-static VALUE rb_coverages = Qundef;
-
 /*
  * call-seq:
  *    Coverage.start  => nil
@@ -22,25 +20,15 @@ static VALUE rb_coverages = Qundef;
 static VALUE
 rb_coverage_start(VALUE klass)
 {
-    if (!RTEST(rb_get_coverages())) {
-	if (rb_coverages == Qundef) {
-	    rb_coverages = rb_hash_new();
-	    rb_obj_hide(rb_coverages);
-	}
-	rb_set_coverages(rb_coverages);
+    VALUE coverages = rb_get_coverages();
+    if (!RTEST(coverages)) {
+	coverages = rb_hash_new();
+	rb_obj_hide(coverages);
+	rb_set_coverages(coverages);
     }
     return Qnil;
 }
 
-/* Make coverage arrays empty so old covered files are no longer tracked. */
-static int
-coverage_clear_result_i(st_data_t key, st_data_t val, st_data_t h)
-{
-    VALUE coverage = (VALUE)val;
-    rb_ary_clear(coverage);
-    return ST_CONTINUE;
-}
-
 static int
 coverage_peek_result_i(st_data_t key, st_data_t val, st_data_t h)
 {
@@ -83,8 +71,6 @@ static VALUE
 rb_coverage_result(VALUE klass)
 {
     VALUE ncoverages = rb_coverage_peek_result(klass);
-    VALUE coverages = rb_get_coverages();
-    st_foreach(RHASH_TBL(coverages), coverage_clear_result_i, ncoverages);
     rb_reset_coverages();
     return ncoverages;
 }
@@ -129,5 +115,4 @@ Init_coverage(void)
     rb_define_module_function(rb_mCoverage, "start", rb_coverage_start, 0);
     rb_define_module_function(rb_mCoverage, "result", rb_coverage_result, 0);
     rb_define_module_function(rb_mCoverage, "peek_result", rb_coverage_peek_result, 0);
-    rb_gc_register_address(&rb_coverages);
 }
diff --git a/test/coverage/test_coverage.rb b/test/coverage/test_coverage.rb
index 1611e17..49b64eb 100644
--- a/test/coverage/test_coverage.rb
+++ b/test/coverage/test_coverage.rb
@@ -63,28 +63,27 @@ def coverage_test_method
 
         File.open("test2.rb", "w") do |f|
           f.puts <<-EOS
-            def coverage_test_method2
-              :ok
-              :ok
-            end
+            1 + 2
           EOS
         end
 
         Coverage.start
         require tmp + '/test.rb'
-        assert_equal 3, Coverage.result[tmp + '/test.rb'].size
+        cov = { "#{tmp}/test.rb" => [1, 0, nil] }
+        assert_equal cov, Coverage.result
 
         # Restart coverage but '/test.rb' is required before restart,
         # so coverage is not recorded.
         Coverage.start
         coverage_test_method
-        assert_equal 0, Coverage.result[tmp + '/test.rb'].size
+        assert_equal({}, Coverage.result)
 
         # Restart coverage and '/test2.rb' is required after restart,
         # so coverage is recorded.
         Coverage.start
         require tmp + '/test2.rb'
-        assert_equal 4, Coverage.result[tmp + '/test2.rb'].size
+        cov = { "#{tmp}/test2.rb" => [1] }
+        assert_equal cov, Coverage.result
       }
     }
   ensure
diff --git a/thread.c b/thread.c
index a68651b..e3efcd3 100644
--- a/thread.c
+++ b/thread.c
@@ -4800,9 +4800,20 @@ rb_set_coverages(VALUE coverages)
     rb_add_event_hook(update_coverage, RUBY_EVENT_COVERAGE, Qnil);
 }
 
+/* Make coverage arrays empty so old covered files are no longer tracked. */
+static int
+reset_coverage_i(st_data_t key, st_data_t val, st_data_t dummy)
+{
+    VALUE coverage = (VALUE)val;
+    rb_ary_clear(coverage);
+    return ST_CONTINUE;
+}
+
 void
 rb_reset_coverages(void)
 {
+    VALUE coverages = rb_get_coverages();
+    st_foreach(rb_hash_tbl_raw(coverages), reset_coverage_i, 0);
     GET_VM()->coverages = Qfalse;
     rb_remove_event_hook(update_coverage);
 }
-- 
2.4.3

