Add Automatic IDNA memory leak detection to CI + keep libidn2 opt-in …

…for the moment

.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:

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

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

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
+  # 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

lib/addressable/idna/libidn2.rb

@@ -51,7 +51,7 @@ def self.to_unicode(value)
         res = idn2_to_unicode_8z8z(value, pointer, IDN2_NONTRANSITIONAL)
         return value if res != 0
         result = pointer.read_pointer.read_string
-        idn2_free(pointer.read_pointer)
+        # idn2_free(pointer.read_pointer)
         result.force_encoding('UTF-8')
       end
     end

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,40 @@ 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
+      100_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
+    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