Skip to content

Commit

Permalink
Use only RUBY_FREE_AT_EXIT on supported Rubies
Browse files Browse the repository at this point in the history
On Ruby 3.4 and later, we can exclusively use RUBY_FREE_AT_EXIT and
disable most of the heuristics. This removes some of the limitations of
the heuristics and makes ruby_memcheck more accurate.

This feature could cause more false-positives, so there is an escape
hatch by passing `use_only_ruby_free_at_exit: false` to the configuration.
  • Loading branch information
peterzhu2118 committed May 2, 2024
1 parent e884801 commit a9da73d
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 2 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ If you want to override any of the default configurations you can call `RubyMemc
- `temp_dir`: Optional. The directory to store temporary files. It defaults to a temporary directory. This is present for development debugging, so you shouldn't have to use it.
- `output_io`: Optional. The `IO` object to output Valgrind errors to. Defaults to standard error.
- `filter_all_errors`: Optional. Whether to filter all kinds of Valgrind errors (not just memory leaks). This feature should only be used if you're encountering a large number of illegal memory accesses coming from Ruby. If you need to use this feature, you may have found a bug inside of Ruby. Consider reporting it to the [Ruby bug tracker](https://bugs.ruby-lang.org/projects/ruby-master/issues/new). Defaults to `false`.
- `use_only_ruby_free_at_exit`: Optional. Use only the [`RUBY_FREE_AT_EXIT`](https://bugs.ruby-lang.org/issues/19993) feature introduced in Ruby 3.3 and disables most of the heuristics inside of ruby_memcheck. Disable this if you want to use the original heuristics. Defaults to `true` for Ruby 3.4 and later, `false` otherwise. Note: while `RUBY_FREE_AT_EXIT` was introduced in Ruby 3.3, there are bugs which prevents it from working well, so it is only enabled by default for Ruby 3.4 and later.

## Suppression files

Expand Down
8 changes: 7 additions & 1 deletion lib/ruby_memcheck/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ class Configuration
/\Arb_thread_create\z/, # Threads are relased to a cache, so they may be reported as a leak
/\Arb_yield/,
].freeze
RUBY_FREE_AT_EXIT_SUPPORTED = Gem::Version.new(RUBY_VERSION) >= Gem::Version.new("3.4.0")

attr_reader :binary_name
attr_reader :ruby
Expand All @@ -43,9 +44,11 @@ class Configuration
attr_reader :loaded_features_file
attr_reader :output_io
attr_reader :filter_all_errors
attr_reader :use_only_ruby_free_at_exit

alias_method :valgrind_generate_suppressions?, :valgrind_generate_suppressions
alias_method :filter_all_errors?, :filter_all_errors
alias_method :use_only_ruby_free_at_exit?, :use_only_ruby_free_at_exit

def initialize(
binary_name: nil,
Expand All @@ -57,7 +60,8 @@ def initialize(
skipped_ruby_functions: DEFAULT_SKIPPED_RUBY_FUNCTIONS,
temp_dir: Dir.mktmpdir,
output_io: $stderr,
filter_all_errors: false
filter_all_errors: false,
use_only_ruby_free_at_exit: RUBY_FREE_AT_EXIT_SUPPORTED
)
@binary_name = binary_name
@ruby = ruby
Expand All @@ -83,6 +87,8 @@ def initialize(
]

@loaded_features_file = Tempfile.create("", @temp_dir)

@use_only_ruby_free_at_exit = use_only_ruby_free_at_exit
end

def command(*args)
Expand Down
26 changes: 26 additions & 0 deletions lib/ruby_memcheck/stack.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,32 @@ def initialize(configuration, loaded_binaries, stack_xml)
end

def skip?
if @configuration.use_only_ruby_free_at_exit?
skip_using_ruby_free_at_exit?
else
skip_using_original_heuristics?
end
end

private

def skip_using_ruby_free_at_exit?
if configuration.binary_name.nil?
false
else
in_binary = false

frames.each do |frame|
if frame.in_binary?
in_binary = true
end
end

!in_binary
end
end

def skip_using_original_heuristics?
in_binary = false

frames.each do |frame|
Expand Down
35 changes: 34 additions & 1 deletion test/ruby_memcheck/shared_test_task_reporter_tests.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,40 @@ def test_does_not_report_uninitialized_value
assert_empty(@output_io.string)
end

def test_call_into_ruby_mem_leak_does_not_report
def test_call_into_ruby_mem_leak_does_not_report_when_RUBY_FREE_AT_EXIT_is_not_supported
skip if Configuration::RUBY_FREE_AT_EXIT_SUPPORTED

ok = run_with_memcheck(<<~RUBY)
RubyMemcheck::CTestOne.new.call_into_ruby_mem_leak
RUBY

assert(ok)
assert_empty(@test_task.reporter.errors)
assert_empty(@output_io.string)
end

def test_call_into_ruby_mem_leak_not_report_when_RUBY_FREE_AT_EXIT_is_supported
skip unless Configuration::RUBY_FREE_AT_EXIT_SUPPORTED

error = assert_raises do
run_with_memcheck(<<~RUBY)
RubyMemcheck::CTestOne.new.call_into_ruby_mem_leak
RUBY
end
assert_equal(RubyMemcheck::TestTaskReporter::VALGRIND_REPORT_MSG, error.message)

output = @output_io.string
assert_equal(1, @test_task.reporter.errors.length, output)

refute_empty(output)
assert_match(/^ \*c_test_one_call_into_ruby_mem_leak \(ruby_memcheck_c_test_one\.c:\d+\)$/, output)
end

def test_call_into_ruby_mem_leak_not_report_when_RUBY_FREE_AT_EXIT_is_supported_but_use_only_ruby_free_at_exit_disabled
skip unless Configuration::RUBY_FREE_AT_EXIT_SUPPORTED

build_configuration(use_only_ruby_free_at_exit: false)

ok = run_with_memcheck(<<~RUBY)
RubyMemcheck::CTestOne.new.call_into_ruby_mem_leak
RUBY
Expand Down

0 comments on commit a9da73d

Please sign in to comment.