From 421cdcaa019b7ecc30f944d8c0131770b71d8829 Mon Sep 17 00:00:00 2001 From: Adrien Rey-Jarthon Date: Tue, 11 Apr 2023 22:59:26 +0200 Subject: [PATCH] Add Automatic IDNA memory leak detection to CI + keep libidn2 opt-in for the moment --- .github/workflows/test.yml | 4 +++ README.md | 10 ++++--- benchmark/idna.rb | 21 --------------- lib/addressable/idna.rb | 20 +++++--------- tasks/profile.rake | 54 +++++++++++++++++++++++++++++++------- 5 files changed, 61 insertions(+), 48 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 08d804a5..886bbe37 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -40,6 +40,10 @@ jobs: Profile Memory Allocation with ${{ matrix.idna_mode }} IDNA during Addressable::Template#match run: bundle exec rake profile:template_match_memory + - name: >- + Test for ${{ matrix.idna_mode }} IDNA backend memory leaks + run: bundle exec rake profile:idna_memory_leak + coverage: runs-on: ${{ matrix.os }} strategy: diff --git a/README.md b/README.md index bd981761..8bc8cad4 100644 --- a/README.md +++ b/README.md @@ -97,9 +97,11 @@ $ gem install addressable # IDNA support (unicode hostnames) Three IDNA implementations are available, the first one available is used: -- A `libidn2` wrapper (if `libidn2` is installed), supporting IDNA2008+UTS#46. - A `libidn1` wrapper (if `libidn` and the `idn` gem are installed), supporting IDNA2003. - A pure ruby implementation (slower), [almost](https://github.com/sporkmonger/addressable/issues/491) supporting IDNA2008. +- A `libidn2` wrapper (if `libidn2` is installed), supporting IDNA2008+UTS#46. + +Note: in the future major version, `libidn2` will become the default. To install `libidn2`: @@ -108,7 +110,7 @@ $ sudo apt-get install libidn2-dev # Debian/Ubuntu $ brew install libidn # OS X ``` -To install the legacy `libidn1` and the `idn` gem (also add it to your Gemfile): +To install `libidn1` and the `idn` gem (also add it to your Gemfile): ```console $ sudo apt-get install libidn11-dev # Debian/Ubuntu @@ -127,8 +129,8 @@ Finally if you want to force a different IDNA implementation, you can do so like ```ruby require "addressable/idna/pure.rb" Addressable::IDNA.backend = Addressable::IDNA::Pure -require "addressable/idna/libidn1" -Addressable::IDNA.backend = Addressable::IDNA::Libidn1 +require "addressable/idna/libidn2" +Addressable::IDNA.backend = Addressable::IDNA::Libidn2 ``` # Semantic Versioning diff --git a/benchmark/idna.rb b/benchmark/idna.rb index 0cadc11a..97cc7b8c 100644 --- a/benchmark/idna.rb +++ b/benchmark/idna.rb @@ -39,24 +39,3 @@ # pure 6.042877 0.000000 6.042877 ( 6.043252) # libidn 0.521668 0.000000 0.521668 ( 0.521704) # libidn2 0.764782 0.000000 0.764782 ( 0.764863) - -puts "\nMemory leak test for libidn2 (memory should stabilize quickly):" -GC.disable # Only run GC when manually called -10.times do - N.times { Addressable::IDNA::Libidn2.to_unicode(Addressable::IDNA::Libidn2.to_ascii(value)) } - GC.start # Run a major GC - pid, size = `ps ax -o pid,rss | grep -E "^[[:space:]]*#{$$}"`.strip.split.map(&:to_i) - puts " Memory: #{size/1024}MB" # show process memory -end - -# Memory leak test for libidn2 (memory should stabilize quickly): -# Memory: 117MB -# Memory: 121MB -# Memory: 121MB -# Memory: 121MB -# Memory: 121MB -# Memory: 121MB -# Memory: 121MB -# Memory: 121MB -# Memory: 121MB -# Memory: 121MB \ No newline at end of file diff --git a/lib/addressable/idna.rb b/lib/addressable/idna.rb index db7f8c2e..fcb7b2d2 100644 --- a/lib/addressable/idna.rb +++ b/lib/addressable/idna.rb @@ -42,17 +42,11 @@ def unicode_normalize_kc(value) end begin - require "addressable/idna/libidn2" - Addressable::IDNA.backend = Addressable::IDNA::Libidn2 + require "addressable/idna/libidn1" + Addressable::IDNA.backend = Addressable::IDNA::Libidn1 rescue LoadError - # libidn2 or the ffi gem was not available, fall back on libidn1 - begin - require "addressable/idna/libidn1" - Addressable::IDNA.backend = Addressable::IDNA::Libidn1 - rescue LoadError - # libidn or the idn gem was not available, fall back on a pure-Ruby - # implementation... - require "addressable/idna/pure" - Addressable::IDNA.backend = Addressable::IDNA::Pure - end -end \ No newline at end of file + # libidn or the idn gem was not available, fall back on a pure-Ruby + # implementation... + require "addressable/idna/pure" + Addressable::IDNA.backend = Addressable::IDNA::Pure +end diff --git a/tasks/profile.rake b/tasks/profile.rake index 1ec75e0a..2e5d9d81 100644 --- a/tasks/profile.rake +++ b/tasks/profile.rake @@ -1,8 +1,19 @@ # frozen_string_literal: true namespace :profile do + task :idna_selection do + require "addressable/idna" + if ENV["IDNA_MODE"] == "pure" + require "addressable/idna/pure" + Addressable::IDNA.backend = Addressable::IDNA::Pure + elsif ENV["IDNA_MODE"] == "libidn2" + require "addressable/idna/libidn2" + Addressable::IDNA.backend = Addressable::IDNA::Libidn2 + end + end + desc "Profile Template match memory allocations" - task :template_match_memory do + task :template_match_memory => :idna_selection do require "memory_profiler" require "addressable/template" @@ -35,16 +46,9 @@ namespace :profile do end desc "Profile URI parse memory allocations" - task :memory do + task :memory => :idna_selection do require "memory_profiler" require "addressable/uri" - if ENV["IDNA_MODE"] == "pure" - require "addressable/idna/pure" - Addressable::IDNA.backend = Addressable::IDNA::Pure - elsif ENV["IDNA_MODE"] == "libidn1" - require "addressable/idna/libidn1" - Addressable::IDNA.backend = Addressable::IDNA::Libidn1 - end start_at = Time.now.to_f report = MemoryProfiler.report do @@ -65,11 +69,41 @@ namespace :profile do puts "Total allocated: #{t_allocated} (#{report.total_allocated} objects)" puts "Total retained: #{t_retained} (#{report.total_retained} objects)" - puts "Took #{end_at - start_at} seconds" + puts "Took #{(end_at - start_at).round(1)} seconds" puts "IDNA backend: #{Addressable::IDNA.backend.name}" FileUtils.mkdir_p("tmp") report.pretty_print(to_file: "tmp/memprof.txt", **print_options) end end + + desc "Test for IDNA backend memory leaks" + task :idna_memory_leak => :idna_selection do + value = "fiᆵリ宠퐱卄.com" + puts "\nMemory leak test for IDNA backend: #{Addressable::IDNA.backend.name}" + start_at = Time.now.to_f + GC.disable # Only run GC when manually called + samples = [] + 10.times do + 50_000.times { + Addressable::IDNA.to_unicode(Addressable::IDNA.to_ascii(value)) + } + GC.start # Run a major GC + _, size = `ps ax -o pid,rss | grep -E "^[[:space:]]*#{$$}"`.strip.split.map(&:to_i) + samples << size/1024 + puts " Memory: #{size/1024}MB" # show process memory + end + end_at = Time.now.to_f + samples.shift # remove first sample which is often unstable in pure ruby + percent = (samples.last - samples.first) * 100 / samples.first + + puts "Took #{(end_at - start_at).round(1)} seconds" + puts "Memory rose from #{samples.first}MB to #{samples.last}MB" + if percent > 10 + puts "Potential MEMORY LEAK detected (#{percent}% increase)" + exit 1 + else + puts "Looks fine (#{percent}% increase)" + end + end end