Merge pull request #2344 from sparklemotion/flavorjones-valgrind-memc…

…heck-experiment

test: try using ruby_memcheck to look for leaks

.github/workflows/ci.yml

@@ -297,3 +297,26 @@ jobs:
             bundle install --local || bundle install
             bundle exec rake compile -- --${{matrix.sys}}-system-libraries
             bundle exec rake test
+
+  memcheck:
+    continue-on-error: true # this is experimental!
+    strategy:
+      fail-fast: false
+      matrix:
+        sys: ["disable"]
+        ruby: ["3.0"]
+    runs-on: ubuntu-latest
+    container:
+      image: ghcr.io/sparklemotion/nokogiri-test:mri-${{matrix.ruby}}
+    steps:
+      - uses: actions/checkout@v2
+        with:
+          submodules: true
+      - uses: actions/cache@v2
+        if: matrix.sys == 'disable'
+        with:
+          path: ports
+          key: ports-ubuntu-${{hashFiles('dependencies.yml', 'patches/**/*.patch')}}
+      - run: bundle install --local || bundle install
+      - run: bundle exec rake compile -- --${{matrix.sys}}-system-libraries
+      - run: bundle exec rake test:memcheck

nokogiri.gemspec

@@ -334,6 +334,7 @@ Gem::Specification.new do |spec|
   spec.add_development_dependency("rubocop-performance", "~> 1.11")
   spec.add_development_dependency("rubocop-rake", "~> 0.6")
   spec.add_development_dependency("rubocop-shopify", "~> 2.3")
+  spec.add_development_dependency("ruby_memcheck", "~> 1.0")
   spec.add_development_dependency("simplecov", "~> 0.20")
 
   spec.extensions << "ext/nokogiri/extconf.rb"

rakelib/test.rake

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require "rake/testtask"
+require "ruby_memcheck"
 
 #
 #  much of this was ripped out of hoe-debugging
@@ -15,6 +16,11 @@ class ValgrindTestTask < Rake::TestTask
                       "--error-exitcode=#{ERROR_EXITCODE}",
                       "--gen-suppressions=all",]
 
+  RubyMemcheck.config(
+    binary_name: "nokogiri",
+    valgrind_generate_suppressions: true,
+  )
+
   def ruby(*args, **options, &block)
     valgrind_options = check_for_suppression_file(VALGRIND_OPTIONS)
     command = "ulimit -s unlimited && valgrind #{valgrind_options.join(" ")} #{RUBY} #{args.join(" ")}"
@@ -96,4 +102,8 @@ namespace "test" do
   LldbTestTask.new("lldb") do |t|
     nokogiri_test_task_configuration(t)
   end
+
+  RubyMemcheck::TestTask.new("memcheck") do |t|
+    nokogiri_test_task_configuration(t)
+  end
 end

scripts/test-gem-build

@@ -24,6 +24,8 @@ fi
 set -x
 
 bundle install --local || bundle install
+bundle list
+bundle list --paths
 bundle exec rake set-version-to-timestamp
 
 if [[ "${BUILD_NATIVE_GEM}" == "ruby" ]] ; then

suppressions/nokogiri_ruby.supp

@@ -8,3 +8,204 @@
    fun:_dl_map_object
    ...
 }
+{
+   this should be caught by default suppressions see #2314
+   Memcheck:Addr16
+   fun:strncmp
+   fun:is_dst
+   fun:_dl_dst_*
+   ...
+   fun:_dl_map_object
+   ...
+}
+{
+  TODO TestHtml5TreeConstructionNoscript01#test_0
+  Memcheck:Leak
+  fun:malloc
+  fun:gumbo_alloc
+  fun:gumbo_strdup
+  fun:handle_markup_declaration_open_state
+  fun:gumbo_lex
+  fun:gumbo_parse_with_options
+  fun:perform_parse
+  fun:parse
+}
+{
+  TODO TestHtml5TreeConstructionNoscript01#test_0
+  Memcheck:Leak
+  fun:malloc
+  fun:gumbo_alloc
+  fun:gumbo_string_buffer_to_string
+  fun:finish_temporary_buffer
+  fun:handle_doctype_name_state
+  fun:gumbo_lex
+  fun:gumbo_parse_with_options
+  fun:perform_parse
+  fun:parse
+}
+{
+  TODO xpath syntax errors raise and don't allow xmlXpathEval to clean up, see #2096 for related work
+  Memcheck:Leak
+  fun:malloc
+  fun:objspace_xmalloc0
+  fun:ruby_xmalloc0
+  fun:ruby_xmalloc_body
+  fun:ruby_xmalloc
+  fun:xmlXPathNewParserContext
+  fun:xmlXPathEval
+  fun:evaluate
+}
+{
+  TODO
+  # 4 bytes in 1 blocks are definitely lost in loss record 1,070 of 37,883
+  # *xmlStrndup (xmlstring.c:45)
+  # *xmlXPathParseNCName (xpath.c:9792)
+  # *xmlXPathCompStep (xpath.c:11325)
+  # *xmlXPathCompRelativeLocationPath (xpath.c:11437)
+  # *xmlXPathCompLocationPath (xpath.c:11489)
+  # *xmlXPathCompPathExpr (xpath.c:10672)
+  # *xmlXPathCompUnionExpr (xpath.c:10703)
+  # *xmlXPathCompUnaryExpr (xpath.c:10743)
+  # *xmlXPathCompMultiplicativeExpr (xpath.c:10768)
+  # *xmlXPathCompAdditiveExpr (xpath.c:10809)
+  # *xmlXPathCompRelationalExpr (xpath.c:10847)
+  # *xmlXPathCompEqualityExpr (xpath.c:10886)
+  # *xmlXPathCompAndExpr (xpath.c:10917)
+  # *xmlXPathCompileExpr (xpath.c:10955)
+  # *xmlXPathEvalExpr (xpath.c:14423)
+  # *xmlXPathEvalExpr (xpath.c:14405)
+  # *xmlXPathEval (xpath.c:14463)
+  # *evaluate (xml_xpath_context.c:322)
+  Memcheck:Leak
+  fun:malloc
+  fun:objspace_xmalloc0
+  fun:ruby_xmalloc0
+  fun:ruby_xmalloc_body
+  fun:ruby_xmalloc
+  fun:xmlStrndup
+  fun:xmlXPathParseNCName
+  ...
+  fun:xmlXPathEval
+  fun:evaluate
+}
+{
+  TODO
+  # 44 bytes in 1 blocks are definitely lost in loss record 14,160 of 37,883
+  # *xmlXPathCompOpEval (xpath.c:13197)
+  # *xmlXPathCompOpEval (xpath.c:12947)
+  # *xmlXPathCompOpEvalToBoolean (xpath.c:13589)
+  # *xmlXPathNodeSetFilter (xpath.c:11664)
+  # *xmlXPathNodeCollectAndTest (xpath.c:12492)
+  # *xmlXPathCompOpEval (xpath.c:13105)
+  # *xmlXPathCompOpEval (xpath.c:12947)
+  # *xmlXPathCompOpEval (xpath.c:13353)
+  # *xmlXPathCompOpEval (xpath.c:12947)
+  # *xmlXPathRunEval (xpath.c:13946)
+  # *xmlXPathEval (xpath.c:14463)
+  # *evaluate (xml_xpath_context.c:322)
+  Memcheck:Leak
+  fun:malloc
+  fun:__vasprintf_internal
+  fun:xpath_generic_exception_handler
+  fun:xmlXPathCompOpEval
+  ...
+  fun:xmlXPathEval
+  fun:evaluate
+}
+{
+  TODO
+  # 96 (16 direct, 80 indirect) bytes in 1 blocks are definitely lost in loss record 24,755 of 37,883
+  # *xmlXPathNodeSetCreate (xpath.c:3564)
+  # *xmlXPathNodeCollectAndTest (xpath.c:12201)
+  # *xmlXPathCompOpEval (xpath.c:13105)
+  # *xmlXPathCompOpEval (xpath.c:12947)
+  # *xmlXPathCompOpEval (xpath.c:13353)
+  # *xmlXPathCompOpEval (xpath.c:12947)
+  # *xmlXPathRunEval (xpath.c:13946)
+  # *xmlXPathEval (xpath.c:14463)
+  # *evaluate (xml_xpath_context.c:322)
+  Memcheck:Leak
+  fun:malloc
+  fun:objspace_xmalloc0
+  fun:ruby_xmalloc0
+  fun:ruby_xmalloc_body
+  fun:ruby_xmalloc
+  fun:xmlXPathNodeSetCreate
+  fun:xmlXPathNodeCollectAndTest
+  fun:xmlXPathCompOpEval
+  ...
+  fun:xmlXPathEval
+  fun:evaluate
+}
+{
+  TODO
+  # 128 bytes in 1 blocks are definitely lost in loss record 26,649 of 37,883
+  # *registr (xslt_stylesheet.c:246)
+  Memcheck:Leak
+  fun:malloc
+  fun:objspace_xmalloc0
+  fun:ruby_xmalloc0
+  fun:ruby_xmalloc_body
+  fun:ruby_xmalloc
+  fun:ar_alloc_table
+  fun:rb_hash_aset
+  fun:registr
+}
+{
+  TODO
+  # 240 (120 direct, 120 indirect) bytes in 1 blocks are definitely lost in loss record 28,980 of 37,883
+  # *xmlNewNodeEatName (tree.c:2299)
+  # *xmlNewDocNodeEatName (tree.c:2374)
+  # *xmlSAX2StartElementNs (SAX2.c:2255)
+  # *xmlParseStartTag2 (parser.c:9658)
+  # *xmlParseElementStart (parser.c:10043)
+  # *xmlParseContentInternal (parser.c:9908)
+  # *xmlParseElement (parser.c:9983)
+  # *xmlParseDocument (parser.c:10821)
+  # *xmlDoRead (parser.c:15167)
+  # *read_memory (xml_document.c:331)
+  Memcheck:Leak
+  fun:malloc
+  fun:objspace_xmalloc0
+  fun:ruby_xmalloc0
+  fun:ruby_xmalloc_body
+  fun:ruby_xmalloc
+  fun:xmlNewNodeEatName
+  fun:xmlNewDocNodeEatName
+  fun:xmlSAX2StartElementNs
+  fun:xmlParseStartTag2
+  fun:xmlParseElementStart
+  fun:xmlParseContentInternal
+  fun:xmlParseElement
+  fun:xmlParseDocument
+  fun:xmlDoRead
+  fun:read_memory
+}
+{
+  TODO
+  # 1,464 (72 direct, 1,392 indirect) bytes in 1 blocks are definitely lost in loss record 35,525 of 37,883
+  # *xmlXPathWrapNodeSet (xpath.c:4386)
+  # *xmlXPathNodeCollectAndTest (xpath.c:12549)
+  # *xmlXPathCompOpEval (xpath.c:13105)
+  # *xmlXPathCompOpEval (xpath.c:12947)
+  # *xmlXPathCompOpEval (xpath.c:13102)
+  # *xmlXPathCompOpEval (xpath.c:12947)
+  # *xmlXPathCompOpEval (xpath.c:13353)
+  # *xmlXPathCompOpEval (xpath.c:12947)
+  # *xmlXPathRunEval (xpath.c:13946)
+  # *xmlXPathEval (xpath.c:14463)
+  # *evaluate (xml_xpath_context.c:322)
+  Memcheck:Leak
+  fun:malloc
+  fun:objspace_xmalloc0
+  fun:ruby_xmalloc0
+  fun:ruby_xmalloc_body
+  fun:ruby_xmalloc
+  fun:xmlXPathWrapNodeSet
+  fun:xmlXPathNodeCollectAndTest
+  ...
+  fun:xmlXPathCompOpEval
+  fun:xmlXPathRunEval
+  fun:xmlXPathEval
+  fun:evaluate
+}

test/helper.rb

@@ -12,6 +12,10 @@
 #   - "stress" - run tests with GC.stress set to true
 # - NOKOGIRI_GC: read more in test/test_memory_leak.rb
 #
+
+# make sure we do one final major before the process exits (for valgrind)
+at_exit { GC.start(full_mark: true) } unless ::RUBY_PLATFORM == "java"
+
 require "simplecov"
 SimpleCov.start do
   add_filter "/test/"