-
Notifications
You must be signed in to change notification settings - Fork 185
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
Autoloader racing on multiple threads #3040
Comments
I think you are seeing #2431. Thanks for the detailed issue and reproduction. |
Didn't get too far, still got confused by the nodes layout in execution. I've used the following code for debugging (safe enough to avoid racing but still threaded so the same-thread-locking guards can be tested): require("thread")
autoload("Foo", "./foo.rb")
Foo.new.bar
Thread.new { Foo.new.bar }.join At the first truffleruby/src/main/java/org/truffleruby/language/constants/GetConstantNode.java Line 124 in 7da015f
GetConstantNode again (couldn't figure out why it needs a second one). This eventually replaces the constant entry for Object :
GetConstantNode and resolve the autoload lock: truffleruby/src/main/java/org/truffleruby/language/constants/GetConstantNode.java Line 129 in 7da015f
Now when we go to the second lookup for truffleruby/src/main/java/org/truffleruby/language/constants/GetConstantNode.java Line 74 in 7da015f
Only had vague ideas how to approach it:
I'm looking for some guidance on a better approach here. |
One idea is for the constant being autoloaded to not set it yet but either store it in a new field of AutoloadConstant or maybe in the RubyConstant#value of the autoload RubyConstant. |
A reliable reproducer: # main.rb
autoload :C, "#{__dir__}/c.rb"
$inside = false
t = Thread.new { p C.new.foo }
Thread.pass until $inside
p C.new.foo
t.join # c.rb
class C
def bar
end
$inside = true
sleep 1
def foo
42
end
end |
I tried an edge case, what if while loading the file we create another thread which tries to access the being-autoloaded constant? Answer: it deadlocks on CRuby: # c.rb
class C
def bar
end
Thread.new { p C }.join
$inside = true
sleep 1
def foo
42
end
end So it's simple, we don't need to handle that case, it can just hang. If we simplify main.rb we can get CRuby to print the deadlock: # main.rb
autoload :C, "#{__dir__}/c.rb"
p C.new.foo gives
|
With Fibers on 3.2, with
So just an early error but still a deadlock. |
I have a fix for this in #3078, I'm still polishing it a bit. I also noticed we still have the notion of "undefined constants", but this should have been removed in CRuby 3.1, so now we should be able to clean that up as well. But probably best done separately, it's already quite a complex change as-is. |
Autoloader use with multiple threads cause premature use of the class.
Example
When executing mutliple
Rails
tests with the debugger:jt --use jvm-ce ruby --jdebug -S ./bin/rails test test/folder
the execution fails with:The test executor spawns threads to spread the test execution, and most threads fail with this error (not all). What's happening is the very first thread is referring to
ActiveSupport::CurrentAttributes
, an unknown constant. The autoload definition in https://github.com/rails/rails/blob/2bfb6565a4996a19065e03300d9061e9eb265d92/activesupport/lib/active_support.rb#L42 delegates the resolution to Zeitwerk and it starts loading the class (snippet from the first thread load backtrace):Meanwhile the other threads will just use the class as if it's already loaded - which is not the case, the class's Ruby code is still executing (function definitions) so some threads won't have all the methods available: hence the error. This was confirmed by logging file execution start/end.
Reproduce
rails generate scaffold PostA name:string title:string content:text value:integer
--jdebug
Minimum example
Unfortunately reproducing it with a minimum example was tricky. It seems using
Module#autolad
vsActiveSupport::Autoload#autoload
was the same. I couldn't replicate it with--jvm-ce
but saw something similar with--native
.Code
Example output (6 failure, 2 success):
Observation
--jvm-ce
does not reproduce the error with the snippet above--jdebug
on the Rails test run because without it there is a different racing error (tracked separately), probably the debugger slows it down enough so the first error does not occur that often--engine.Compilation=false
)The text was updated successfully, but these errors were encountered: