-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
Java::JavaLang::StackOverflowError running spec which works on 2.2.1 #3193
Comments
Confirmed on master. Investigating. |
This is taking a long time to sort out...may not be able to get this in for 9.0.2. |
Still failing on master. |
FYI, the prepend issue is not related; 9.0.0.0.rc2 did not have prepend implemented yet. |
If I log the block being invoked recursively here, I see two line numbers over and over once it starts to go down the rabbit hole:
The one at line 720 looks pretty mundane, and probably isn't the culprit: # Returns the configuration option for color, but should not
# be used to check if color is supported.
#
# @see color_enabled?
# @return [Boolean]
def color
value_for(:color) { @color }
end The other one at line 1802, however, contains a yield within a block, for which we've had a few sneaky bugs creep in: def value_for(key)
@preferred_options.fetch(key) { yield }
end The @JRubyMethod
public IRubyObject fetch(ThreadContext context, IRubyObject key, Block block) {
Ruby runtime = context.runtime;
IRubyObject value = internalGet(key);
if (value == null) {
if (block.isGiven()) return block.yield(context, key);
throw runtime.newKeyError("key not found: :" + key);
}
return value;
} Turning off JIT does not appear to fix the issue. Curiouser and curiouser. |
Ok, there's a possibility prepend is involved, since the example script prepends a define_method method into the example group, and the define_method does a super call. I may be able to reduce this. |
Ok, yep, prepend is involved. If I modify the prepend section to instead do an include on the singleton of @example_group.instance_eval do
self.class.send :include, observation_module
before { let_initialized = false }
after { block.call if let_initialized }
end |
With a somewhat reduced example, I get this Ruby backtrace for the recursing bit:
Reduced version: module LetAndAfterExtensions
def let(name, &block)
super(name, &block)
observation_module = Module.new do
define_method(name) do |*args|
super(*args)
end
end
class_exec do
prepend observation_module
end
end
end
RSpec.configure do |c|
c.extend LetAndAfterExtensions
end
RSpec.describe "some thing" do
let(:foo) { 2 } #.and_after { }
it "does something" do
foo
end
end |
I believe I have a reduced reproduction: class A
define_method :foo do
super
end
prepend Module.new do
def foo
super
end
end
end
A.new.foo Interestingly, this only fails if run in the interpreter, either simple or full. If run in the JIT (as when running directly at command line) it works fine. It's likely that the original rspec example would have started working had it not reached a stack overflow before the method bodies in question had jitted. So this seems like an issue with unresolved super in a define_method not starting from the proper place. |
It turns out the contents of the prepended module are not important:
|
A slightly longer example, using a concrete module rather than a block-initialized module, shows we bounce back and forth between the prepended
|
An rspec extension I'm trying to get working works fine on 2.2.1, but gives a stack overflow on JRuby 9.0.0.0. Demonstration:
my_spec.rb cut down to just the extension we are trying to get working, plus one spec to call the code which triggers the issue:
Same spec on 2.2.1:
In our actual specs, we don't get this stack overflow, but instead get an error about the
prepend
method not being found onClass
. This stack overflow is what I found when trying to cut that problem down to a small enough size to report. So this issue is either related, or you will discover that other issue once this one is fixed.The issue occurred on 9.0.0.0.rc2 as well, but I haven't tested any earlier versions.
The text was updated successfully, but these errors were encountered: