[ruby-core:125931] [Ruby Bug#22179] Coverage: redefined methods silently disappear from method Coverage.result after GC
Issue #22179 has been reported by sferik (Erik Berlin). ---------------------------------------- Bug #22179: Coverage: redefined methods silently disappear from method Coverage.result after GC https://bugs.ruby-lang.org/issues/22179 * Author: sferik (Erik Berlin) * Status: Open * Backport: 3.3: UNKNOWN, 3.4: UNKNOWN, 4.0: UNKNOWN ---------------------------------------- When method coverage is enabled, a method that has been redefined vanishes from `Coverage.result` once GC runs. It is not reported as uncovered (count 0), it is missing altogether, so the set of methods reported for a file changes nondeterministically depending on GC timing. ##### Reproduction target.rb: ```ruby def foo 1 end def foo 2 end foo ``` run.rb: ```ruby require "coverage" Coverage.start(methods: true) require_relative "./target" GC.start if ENV["FORCE_GC"] pp Coverage.result.find { |f, _| f.end_with?("target.rb") }.last[:methods] ``` Without GC, both definitions are reported: ``` $ ruby run.rb {[Object, :foo, 5, 0, 7, 3] => 1, [Object, :foo, 1, 0, 3, 3] => 0} ``` With a GC before `Coverage.result`, the first definition is gone: ``` $ FORCE_GC=1 ruby run.rb {[Object, :foo, 5, 0, 7, 3] => 1} ``` The explicit `GC.start` is only for determinism. In real programs any GC that happens to run between the redefinition and `Coverage.result` has the same effect, which is how this was noticed in the wild. I’ve looked into why this is happening and I believe it's because method coverage is not stored per file the way line and branch coverage are, it's reconstructed at result time by walking the heap for method entries (`method_coverage_i` via `rb_objspace_each_objects` in ext/coverage/coverage.c). A shadowed definition that was never called has no entry in `me2counter` and exists only as a garbage method entry after redefinition, so once GC collects it, the heap walk cannot see it and the definition is silently forgotten. This issue was discovered when it was reported as [a bug in SimpleCov](https://github.com/simplecov-ruby/simplecov/issues/1217), of which I am a maintainer. Reproduced on 3.2.11, 3.3.11, 3.4.10, and 4.0.5. -- https://bugs.ruby-lang.org/
Issue #22179 has been updated by mame (Yusuke Endoh). Thanks for the report and the very precise analysis. I think your diagnosis is exactly right. After discussing with @ko1, we decided to fix it as follows: 1. Record the set of methods when a method is added. 2. At `Coverage.result` time, stop using `each_object`, and report method coverage by aggregating that set together with the counter. I've prepared a draft PR. However, @ko1 is planning to implement Ractor-local GC soon, and this change could interfere with it, so he asked to hold off on merging for about a month. https://github.com/ruby/ruby/pull/17729 ---------------------------------------- Bug #22179: Coverage: redefined methods silently disappear from method Coverage.result after GC https://bugs.ruby-lang.org/issues/22179#change-117951 * Author: sferik (Erik Berlin) * Status: Assigned * Assignee: mame (Yusuke Endoh) * Backport: 3.3: UNKNOWN, 3.4: UNKNOWN, 4.0: UNKNOWN ---------------------------------------- When method coverage is enabled, a method that has been redefined vanishes from `Coverage.result` once GC runs. It is not reported as uncovered (count 0), it is missing altogether, so the set of methods reported for a file changes nondeterministically depending on GC timing. ##### Reproduction target.rb: ```ruby def foo 1 end def foo 2 end foo ``` run.rb: ```ruby require "coverage" Coverage.start(methods: true) require_relative "./target" GC.start if ENV["FORCE_GC"] pp Coverage.result.find { |f, _| f.end_with?("target.rb") }.last[:methods] ``` Without GC, both definitions are reported: ``` $ ruby run.rb {[Object, :foo, 5, 0, 7, 3] => 1, [Object, :foo, 1, 0, 3, 3] => 0} ``` With a GC before `Coverage.result`, the first definition is gone: ``` $ FORCE_GC=1 ruby run.rb {[Object, :foo, 5, 0, 7, 3] => 1} ``` The explicit `GC.start` is only for determinism. In real programs any GC that happens to run between the redefinition and `Coverage.result` has the same effect, which is how this was noticed in the wild. I’ve looked into why this is happening and I believe it's because method coverage is not stored per file the way line and branch coverage are, it's reconstructed at result time by walking the heap for method entries (`method_coverage_i` via `rb_objspace_each_objects` in ext/coverage/coverage.c). A shadowed definition that was never called has no entry in `me2counter` and exists only as a garbage method entry after redefinition, so once GC collects it, the heap walk cannot see it and the definition is silently forgotten. This issue was discovered when it was reported as [a bug in SimpleCov](https://github.com/simplecov-ruby/simplecov/issues/1217), of which I am a maintainer. Reproduced on 3.2.11, 3.3.11, 3.4.10, and 4.0.5. -- https://bugs.ruby-lang.org/
Issue #22179 has been updated by sferik (Erik Berlin). Thanks for the quick response and proposed fix. I left one minor note on your patch but overall it looks good to me. Waiting a month for the Ractor-local GC work is fine. Do you know whether this fix will land in 4.1.0 or a patch release before then (e.g. 4.0.6)? Thanks again to you and @ko1! ---------------------------------------- Bug #22179: Coverage: redefined methods silently disappear from method Coverage.result after GC https://bugs.ruby-lang.org/issues/22179#change-117964 * Author: sferik (Erik Berlin) * Status: Assigned * Assignee: mame (Yusuke Endoh) * Backport: 3.3: UNKNOWN, 3.4: UNKNOWN, 4.0: UNKNOWN ---------------------------------------- When method coverage is enabled, a method that has been redefined vanishes from `Coverage.result` once GC runs. It is not reported as uncovered (count 0), it is missing altogether, so the set of methods reported for a file changes nondeterministically depending on GC timing. ##### Reproduction target.rb: ```ruby def foo 1 end def foo 2 end foo ``` run.rb: ```ruby require "coverage" Coverage.start(methods: true) require_relative "./target" GC.start if ENV["FORCE_GC"] pp Coverage.result.find { |f, _| f.end_with?("target.rb") }.last[:methods] ``` Without GC, both definitions are reported: ``` $ ruby run.rb {[Object, :foo, 5, 0, 7, 3] => 1, [Object, :foo, 1, 0, 3, 3] => 0} ``` With a GC before `Coverage.result`, the first definition is gone: ``` $ FORCE_GC=1 ruby run.rb {[Object, :foo, 5, 0, 7, 3] => 1} ``` The explicit `GC.start` is only for determinism. In real programs any GC that happens to run between the redefinition and `Coverage.result` has the same effect, which is how this was noticed in the wild. I’ve looked into why this is happening and I believe it's because method coverage is not stored per file the way line and branch coverage are, it's reconstructed at result time by walking the heap for method entries (`method_coverage_i` via `rb_objspace_each_objects` in ext/coverage/coverage.c). A shadowed definition that was never called has no entry in `me2counter` and exists only as a garbage method entry after redefinition, so once GC collects it, the heap walk cannot see it and the definition is silently forgotten. This issue was discovered when it was reported as [a bug in SimpleCov](https://github.com/simplecov-ruby/simplecov/issues/1217), of which I am a maintainer. Reproduced on 3.2.11, 3.3.11, 3.4.10, and 4.0.5. -- https://bugs.ruby-lang.org/
participants (2)
-
mame (Yusuke Endoh) -
sferik (Erik Berlin)