-
-
Notifications
You must be signed in to change notification settings - Fork 942
Description
This bug actually seems to have already been fixed by @enebo in 129bd63 but there doesn't seem to be any issue ticket for it. To make referring to this issue easier (and to ensure that it will show up in release notes) I'm retroactively filing this bug report for it. Please feel free to immediately close it as fixed.
A bug in JRuby 9.4.0.0 JIT can cause keyword arguments to be passed incorrectly to a method called immediately after an argumentless call to a JITted method that could take keyword arguments (e.g. because it uses argument forwarding). In this case the keyword arguments given to the second method are not received properly, and are effectively turned into an extra positional hash argument (because a flag indicating that "this method call includes no keyword arguments" doesn't get properly reset after the first call).
This error typically manifests as an unexpected ArgumentError when calling a method that expects keyword arguments. However, if the called method can also take extra positional arguments, the call may succeed but the arguments passed to the method will not be what the caller intended them to be. (I've seen this happen with RSpec mocks, which accept any arguments before testing them against the defined expectations.)
From the description of the linked commit:
This was triggering a case like:
class Foo def type "hoooooo" end def one(...) type(...) # This call is forwarding KEYWORD_EMPTY and it is not getting # reset back to call two below. This works in interpreter. end def two(name: nil, del: :taco) puts "WORKS" end def foo two(name: "#{one}[]", del: :taco) end end Foo.new.fooSo 'type' was receiving KEYWORD_EMPTY but because recv_kwargs was never
getting called (or equivalent of what JIT does) this meant two was still
passing an KEYWORD_EMPTY. KEYWORD_EMPTY is a special flag value that
is dynamically determined leading up to a call so it can leak like in
this example if we do not clear it.
FWIW, the line that ended up consistently triggering this bug for us during Rails startup was this line in our config/initializers/session_store.rb:
Backend::Application.config.session_store :cookie_store, :key => '_pc_session'Here, the config method was defined using Module#delegate and its effective definition thus used arguments forwarding, and (apparently) the method also happened to have been called enough in earlier initialization code to always be JITted at this point, causing the initialization to crash with the following error:
org.jruby.exceptions.ArgumentError: (ArgumentError) wrong number of arguments (given 2, expected 0..1)
at RUBY.session_store(/builds/DevHEL/planning-cloud/.relex-gems/9.4.0.0/gems/railties-6.1.7/lib/rails/application/configuration.rb:311)
at RUBY.<main>(/builds/DevHEL/planning-cloud/backend/config/initializers/session_store.rb:1)
...
I managed to temporarily work around the error (for this particular line) by changing the code to:
Backend::Application.config.method(:session_store).call(:cookie_store, :key => '_pc_session')However, this is obviously not a general workaround (although it did let me start the application and continue with further testing) and the issue can and does manifest randomly in other parts of our codebase. For example, as noted above, it has led to intermittent unit test failures due to incorrect arguments getting passed to RSpec mock doubles, with errors such as:
#<Double (anonymous)> received :call with unexpected arguments
expected: ({:some=>:hash}) (keyword arguments)
got: ({:some=>:hash}) (options hash)
Obviously we will need to wait for the next JRuby release before upgrading.