Skip to content

[Bug #21394] Fix memory leak in Prism's RubyVM::InstructionSequence.new #13496

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

peterzhu2118
Copy link
Member

There are two ways to make RubyVM::InstructionSequence.new raise which would cause the options->scopes to leak memory:

  1. Passing in any (non T_FILE) object where the to_str raises.
  2. Passing in a T_FILE object where String#initialize_dup raises. This is because rb_io_path dups the string.

Example 1:

10.times do
  100_000.times do
    RubyVM::InstructionSequence.new(nil)
  rescue TypeError
  end

  puts `ps -o rss= -p #{$$}`
end

Before:

13392
17104
20256
23920
27264
30432
33584
36752
40032
43232

After:

9392
11072
11648
11648
11648
11712
11712
11712
11744
11744

Example 2:

require "tempfile"

MyError = Class.new(StandardError)
String.prepend(Module.new do
  def initialize_dup(_)
    if $raise_on_dup
      raise MyError
    else
      super
    end
  end
end)

Tempfile.create do |f|
  10.times do
    100_000.times do
      $raise_on_dup = true
      RubyVM::InstructionSequence.new(f)
    rescue MyError
    else
      raise "MyError was not raised during RubyVM::InstructionSequence.new"
    end

    puts `ps -o rss= -p #{$$}`
  ensure
    $raise_on_dup = false
  end
end

Before:

14080
18512
22000
25184
28320
31600
34736
37904
41088
44256

After:

12016
12464
12880
12880
12880
12912
12912
12912
12912
12912

[Bug #21394]

There are two ways to make RubyVM::InstructionSequence.new raise which
would cause the options->scopes to leak memory:

1. Passing in any (non T_FILE) object where the to_str raises.
2. Passing in a T_FILE object where String#initialize_dup raises. This is
   because rb_io_path dups the string.

Example 1:

    10.times do
      100_000.times do
        RubyVM::InstructionSequence.new(nil)
      rescue TypeError
      end

      puts `ps -o rss= -p #{$$}`
    end

Before:

    13392
    17104
    20256
    23920
    27264
    30432
    33584
    36752
    40032
    43232

After:

    9392
    11072
    11648
    11648
    11648
    11712
    11712
    11712
    11744
    11744

Example 2:

    require "tempfile"

    MyError = Class.new(StandardError)
    String.prepend(Module.new do
      def initialize_dup(_)
        if $raise_on_dup
          raise MyError
        else
          super
        end
      end
    end)

    Tempfile.create do |f|
      10.times do
        100_000.times do
          $raise_on_dup = true
          RubyVM::InstructionSequence.new(f)
        rescue MyError
        else
          raise "MyError was not raised during RubyVM::InstructionSequence.new"
        end

        puts `ps -o rss= -p #{$$}`
      ensure
        $raise_on_dup = false
      end
    end

Before:

    14080
    18512
    22000
    25184
    28320
    31600
    34736
    37904
    41088
    44256

After:

    12016
    12464
    12880
    12880
    12880
    12912
    12912
    12912
    12912
    12912
Copy link

launchable-app bot commented Jun 2, 2025

Tests Failed

✖️no tests failed ✔️61840 tests passed(1 flake)

@peterzhu2118 peterzhu2118 merged commit 34b407a into ruby:master Jun 3, 2025
83 checks passed
@peterzhu2118 peterzhu2118 deleted the prism-iseq-new-raise-mem-leak branch June 3, 2025 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants