Skip to content

Commit 5225cd7

Browse files
Watson1978claude
andcommitted
Add Valgrind memcheck (ruby_memcheck) and run it in CI
Running the suite under Valgrind on a Ruby C extension is normally impractical: the interpreter itself produces a large volume of reports that have nothing to do with the extension. ruby_memcheck wraps Valgrind and only surfaces errors whose stack trace passes through the extension's .so, which makes the output usable. It is the same tool Shopify runs on nokogiri and liquid-c. rake spec:valgrind This reworks #147, which was reverted in #150 because it broke CI: the Gemfile guarded the dependency on Linux but not on the Ruby version, and ruby_memcheck 3.x requires Ruby >= 3.0, so `bundle install` failed on the 2.7 entry of the matrix. The guard now covers both. Verified on Ruby 2.7.8 locally: bundle install, rake compile and rspec all pass, and the Rakefile simply does not define the task there. The memcheck run gets its own workflow rather than a job inside Ruby.yml, so a report from it cannot turn the main test matrix red. It pins one Ruby, installs Valgrind, and carries a 30 minute job timeout; a cold run -- extension build plus the suite under memcheck -- takes about 1m45s locally. The suite reports nothing on the current tree, so no suppression file is needed. The task does have teeth: run against the use-after-free of a stream's borrowed CDict/DDict it reports 113 Invalid read records, and against the ZSTD_DCtx leak it reports the context as definitely lost, both attributed inside zstdruby.so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 2f033eb commit 5225cd7

3 files changed

Lines changed: 69 additions & 0 deletions

File tree

.github/workflows/valgrind.yml

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
name: Valgrind
2+
3+
on:
4+
push:
5+
branches:
6+
- main
7+
pull_request:
8+
9+
permissions:
10+
contents: read
11+
12+
concurrency:
13+
group: ${{ github.workflow }}-${{ github.ref }}
14+
cancel-in-progress: true
15+
16+
jobs:
17+
valgrind:
18+
name: memcheck (ubuntu, ruby ${{ matrix.ruby }})
19+
runs-on: ubuntu-latest
20+
timeout-minutes: 30
21+
strategy:
22+
fail-fast: false
23+
matrix:
24+
ruby:
25+
- '4.0'
26+
27+
steps:
28+
- uses: actions/checkout@v6
29+
- name: Install Valgrind
30+
run: |
31+
sudo apt-get update
32+
sudo apt-get install -y valgrind
33+
- name: Set up Ruby
34+
uses: ruby/setup-ruby@v1
35+
with:
36+
ruby-version: ${{ matrix.ruby }}
37+
bundler-cache: true
38+
- name: Run the spec suite under Valgrind memcheck
39+
run: bundle exec rake spec:valgrind

Gemfile

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,7 @@ source 'https://rubygems.org'
22

33
# Specify your gem's dependencies in zstd_ruby.gemspec
44
gemspec
5+
6+
if RUBY_PLATFORM.include?('linux') && Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('3.0.0')
7+
gem 'ruby_memcheck', '~> 3.0'
8+
end

Rakefile

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,32 @@ end
1515

1616
task :default => [:clobber, :compile, :spec]
1717

18+
begin
19+
require 'ruby_memcheck'
20+
require 'ruby_memcheck/rspec/rake_task'
21+
22+
RubyMemcheck.config(
23+
binary_name: 'zstdruby',
24+
# Valgrind and YJIT interfere with each other, adding noise and slowdown,
25+
# so keep YJIT disabled while running under Valgrind.
26+
ruby: "#{FileUtils::RUBY} --disable-yjit"
27+
)
28+
29+
namespace :spec do
30+
task :check_valgrind do
31+
unless system('command -v valgrind > /dev/null 2>&1')
32+
abort("\nValgrind is required for `rake spec:valgrind` but was not found.\n" \
33+
"Install it first (Linux only), e.g. `sudo apt-get install valgrind`.\n")
34+
end
35+
end
36+
37+
RubyMemcheck::RSpec::RakeTask.new(valgrind: [:check_valgrind, :compile])
38+
end
39+
rescue LoadError
40+
# ruby_memcheck is an optional development dependency, absent on the platforms
41+
# and Ruby versions the Gemfile excludes. Skip the task instead of breaking.
42+
end
43+
1844
desc 'Sync zstd libs dirs to ext/zstdruby/libzstd'
1945
task :zstd_update do
2046
FileUtils.rm_r("ext/zstdruby/libzstd")

0 commit comments

Comments
 (0)