-
Notifications
You must be signed in to change notification settings - Fork 14
Test all ractors multi ractor #698
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
base: master
Are you sure you want to change the base?
Conversation
This tests for ractor safety issues as well as issues with GC and the ractor scheduler. It also tests move/copy logic because it uses these internally for running the tests. You can enable these tests with RUBY_TESTS_WITH_RACTORS=1 when running `make test-all TESTS=test/ruby`. These ractor tests are currently only working for tests under the "test/ruby" directory, so don't run any others with this ENV var set or you will get errors. Currently there are GC issues with ractors so in tool/lib/test/unit.rb, I disable the GC before running any of the tests (for now). If you uncomment this, you will get errors and you can debug the GC issues.
if main_ractor? | ||
assert_warn(/#{tlhInganHol}/) { | ||
EnvUtil.with_default_internal(nil) { | ||
open(IO::NULL, "w:bom|#{tlhInganHol}") {|f| enc = f.external_encoding} |
Check failure
Code scanning / CodeQL
Use of `Kernel.open` or `IO.read` or similar sinks with a non-constant value Critical test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 days ago
To fix the problem, replace the use of open
(which refers to Kernel.open
) with File.open
for file operations. In this case, change open(IO::NULL, "w:bom|#{tlhInganHol}")
to File.open(IO::NULL, "w:bom|#{tlhInganHol}")
. This ensures that the file is opened safely, and the mode string is only interpreted as a file mode, not as a potential shell command. No additional imports or definitions are needed, as File
is part of Ruby's core library. Only the flagged line (2308) needs to be changed.
-
Copy modified line R2308
@@ -2305,7 +2305,7 @@ | ||
if main_ractor? | ||
assert_warn(/#{tlhInganHol}/) { | ||
EnvUtil.with_default_internal(nil) { | ||
open(IO::NULL, "w:bom|#{tlhInganHol}") {|f| enc = f.external_encoding} | ||
File.open(IO::NULL, "w:bom|#{tlhInganHol}") {|f| enc = f.external_encoding} | ||
} | ||
} | ||
assert_nil(enc) |
assert_no_match(/Amiga/, line) | ||
assert_no_match(/paper/, line) | ||
if main_ractor? | ||
tmp = open(tmpfilename, "r") |
Check failure
Code scanning / CodeQL
Use of `Kernel.open` or `IO.read` or similar sinks with a non-constant value Critical test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 days ago
The best way to fix this problem is to replace all instances of open(tmpfilename, ...)
with File.open(tmpfilename, ...)
. This ensures that the file is opened directly, and Ruby will not interpret the filename as a shell command, even if it starts with a |
. The change should be made in all places within the shown code where open
is called with a variable filename. No additional imports or method definitions are needed, as File
is part of Ruby's core library.
-
Copy modified line R10 -
Copy modified line R18 -
Copy modified line R29 -
Copy modified line R38 -
Copy modified line R63
@@ -7,7 +7,7 @@ | ||
Dir.mktmpdir("ruby_while_tmp") {|tmpdir| | ||
tmpfilename = "#{tmpdir}/ruby_while_tmp.#{$$}" | ||
|
||
tmp = open(tmpfilename, "w") | ||
tmp = File.open(tmpfilename, "w") | ||
tmp.print "tvi925\n"; | ||
tmp.print "tvi920\n"; | ||
tmp.print "vt100\n"; | ||
@@ -15,7 +15,7 @@ | ||
tmp.print "paper\n"; | ||
tmp.close | ||
|
||
tmp = open(tmpfilename, "r") | ||
tmp = File.open(tmpfilename, "r") | ||
assert_instance_of(File, tmp) | ||
|
||
while line = tmp.gets() | ||
@@ -26,7 +26,7 @@ | ||
assert_match(/vt100/, line) | ||
tmp.close | ||
|
||
tmp = open(tmpfilename, "r") | ||
tmp = File.open(tmpfilename, "r") | ||
while line = tmp.gets() | ||
next if /vt100/ =~ line | ||
assert_no_match(/vt100/, line) | ||
@@ -35,7 +35,7 @@ | ||
assert_no_match(/vt100/, line) | ||
tmp.close | ||
|
||
tmp = open(tmpfilename, "r") | ||
tmp = File.open(tmpfilename, "r") | ||
while line = tmp.gets() | ||
lastline = line | ||
line = line.gsub(/vt100/, 'VT100') | ||
@@ -60,7 +60,7 @@ | ||
assert_equal(220, sum) | ||
|
||
if main_ractor? | ||
tmp = open(tmpfilename, "r") | ||
tmp = File.open(tmpfilename, "r") | ||
while line = tmp.gets() | ||
break if $. == 3 | ||
assert_no_match(/vt100/, line) |
Without a VM Lock, there's an unlocked `rb_id_table_delete` for the class's const_tbl which can cause problems. Example: ```ruby class C CONSTANT = 3 end $VERBOSE = nil rs = [] 100.times do rs << Ractor.new do 10_000.times do if defined?(C::CONSTANT) C.send(:remove_const, :CONSTANT) rescue NameError else C.send(:const_set, :CONSTANT, 3) end end end end while rs.any? r, obj = Ractor.select(*rs) rs.delete(r) end ``` Without lock: ../ruby-release/test.rb:14: [BUG] Segmentation fault at 0x0000000000000001 -- Control frame information ----------------------------------------------- miniruby(82790,0x16f49f000) malloc: *** error for object 0x600000f880a0: pointer being freed was not allocated miniruby(82790,0x16f49f000) malloc: *** set a breakpoint in malloc_error_break to debug
Make sure VM lock is not held when calling `load_transcoder_entry`, as that causes deadlock inside ractors. `String#encode` now works inside ractors, among others. Atomic load the rb_encoding_list Without this, wbcheck would sometimes hit a missing write barrier. Co-authored-by: John Hawthorn <[email protected]> Hold VM lock when iterating over global_enc_table.names This st_table can be inserted into at runtime when autoloading encodings. minor optimization when calling Encoding.list
4a3a2d7
to
f3aa1ea
Compare
No description provided.