Skip to content

Commit c397c2d

Browse files
committed
merge revision(s) 34b407a: [Backport #21394]
Fix memory leak in Prism's RubyVM::InstructionSequence.new [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
1 parent acb19e8 commit c397c2d

File tree

3 files changed

+64
-6
lines changed

3 files changed

+64
-6
lines changed

iseq.c

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1319,6 +1319,15 @@ pm_iseq_compile_with_option(VALUE src, VALUE file, VALUE realpath, VALUE line, V
13191319
ln = NUM2INT(line);
13201320
StringValueCStr(file);
13211321

1322+
bool parse_file = false;
1323+
if (RB_TYPE_P(src, T_FILE)) {
1324+
parse_file = true;
1325+
src = rb_io_path(src);
1326+
}
1327+
else {
1328+
src = StringValue(src);
1329+
}
1330+
13221331
pm_parse_result_t result = { 0 };
13231332
pm_options_line_set(&result.options, NUM2INT(line));
13241333
pm_options_scopes_init(&result.options, 1);
@@ -1341,16 +1350,15 @@ pm_iseq_compile_with_option(VALUE src, VALUE file, VALUE realpath, VALUE line, V
13411350
VALUE script_lines;
13421351
VALUE error;
13431352

1344-
if (RB_TYPE_P(src, T_FILE)) {
1345-
VALUE filepath = rb_io_path(src);
1346-
error = pm_load_parse_file(&result, filepath, ruby_vm_keep_script_lines ? &script_lines : NULL);
1347-
RB_GC_GUARD(filepath);
1353+
if (parse_file) {
1354+
error = pm_load_parse_file(&result, src, ruby_vm_keep_script_lines ? &script_lines : NULL);
13481355
}
13491356
else {
1350-
src = StringValue(src);
13511357
error = pm_parse_string(&result, src, file, ruby_vm_keep_script_lines ? &script_lines : NULL);
13521358
}
13531359

1360+
RB_GC_GUARD(src);
1361+
13541362
if (error == Qnil) {
13551363
int error_state;
13561364
iseq = pm_iseq_new_with_opt(&result.node, name, file, realpath, ln, NULL, 0, ISEQ_TYPE_TOP, &option, &error_state);

test/ruby/test_iseq.rb

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,56 @@ def test_invalid_source
297297
assert_raise(TypeError, bug11159) {compile(1)}
298298
end
299299

300+
def test_invalid_source_no_memory_leak
301+
# [Bug #21394]
302+
assert_no_memory_leak(["-rtempfile"], "#{<<-"begin;"}", "#{<<-'end;'}", rss: true)
303+
code = proc do |t|
304+
RubyVM::InstructionSequence.new(nil)
305+
rescue TypeError
306+
else
307+
raise "TypeError was not raised during RubyVM::InstructionSequence.new"
308+
end
309+
310+
10.times(&code)
311+
begin;
312+
1_000_000.times(&code)
313+
end;
314+
315+
# [Bug #21394]
316+
# RubyVM::InstructionSequence.new calls rb_io_path, which dups the string
317+
# and can leak memory if the dup raises
318+
assert_no_memory_leak(["-rtempfile"], "#{<<-"begin;"}", "#{<<-'end;'}", rss: true)
319+
MyError = Class.new(StandardError)
320+
String.prepend(Module.new do
321+
def initialize_dup(_)
322+
if $raise_on_dup
323+
raise MyError
324+
else
325+
super
326+
end
327+
end
328+
end)
329+
330+
code = proc do |t|
331+
Tempfile.create do |f|
332+
$raise_on_dup = true
333+
t.times do
334+
RubyVM::InstructionSequence.new(f)
335+
rescue MyError
336+
else
337+
raise "MyError was not raised during RubyVM::InstructionSequence.new"
338+
end
339+
ensure
340+
$raise_on_dup = false
341+
end
342+
end
343+
344+
code.call(100)
345+
begin;
346+
code.call(1_000_000)
347+
end;
348+
end
349+
300350
def test_frozen_string_literal_compile_option
301351
$f = 'f'
302352
line = __LINE__ + 2

version.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
# define RUBY_VERSION_MINOR RUBY_API_VERSION_MINOR
1212
#define RUBY_VERSION_TEENY 4
1313
#define RUBY_RELEASE_DATE RUBY_RELEASE_YEAR_STR"-"RUBY_RELEASE_MONTH_STR"-"RUBY_RELEASE_DAY_STR
14-
#define RUBY_PATCHLEVEL 38
14+
#define RUBY_PATCHLEVEL 39
1515

1616
#include "ruby/version.h"
1717
#include "ruby/internal/abi.h"

0 commit comments

Comments
 (0)