Skip to content
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

Stack overflow due to RubyGems warn patch #5997

Closed
headius opened this issue Dec 11, 2019 · 1 comment
Closed

Stack overflow due to RubyGems warn patch #5997

headius opened this issue Dec 11, 2019 · 1 comment

Comments

@headius
Copy link
Member

headius commented Dec 11, 2019

RubyGems 3.0 patches Kernel#warn to do some uplevel-related stack trace manipulation. It's not clear to me the purpose of this code, but it seems to trigger a stack overflow for us with the following trace:

	at blah.RUBY$method$warn$1(blah.rb:3)
	at org.jruby.internal.runtime.methods.CompiledIRMethod.call(CompiledIRMethod.java:173)
	at org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:172)
	at org.jruby.RubyKernel.warnStr(RubyKernel.java:1276)
	at org.jruby.RubyKernel.warnObj(RubyKernel.java:1263)
	at org.jruby.RubyKernel.warn(RubyKernel.java:1253)
	at org.jruby.RubyKernel$INVOKER$s$warn.call(RubyKernel$INVOKER$s$warn.gen)
	at org.jruby.internal.runtime.methods.JavaMethod$JavaMethodN.call(JavaMethod.java:833)
	at org.jruby.RubyMethod.call(RubyMethod.java:132)
	at org.jruby.RubyMethod$INVOKER$i$call.call(RubyMethod$INVOKER$i$call.gen)
	at org.jruby.internal.runtime.methods.DynamicMethod.call(DynamicMethod.java:174)
	at org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:74)
	at Users.headius.projects.jruby.lib.ruby.stdlib.rubygems.core_ext.kernel_warn.invokeOther13:call(/Users/headius/projects/jruby/lib/ruby/stdlib/rubygems/core_ext/kernel_warn.rb:17)
	at Users.headius.projects.jruby.lib.ruby.stdlib.rubygems.core_ext.kernel_warn.RUBY$block$Kernel$2(/Users/headius/projects/jruby/lib/ruby/stdlib/rubygems/core_ext/kernel_warn.rb:17)
	at org.jruby.runtime.CompiledIRBlockBody.callDirect(CompiledIRBlockBody.java:136)
	at org.jruby.runtime.IRBlockBody.call(IRBlockBody.java:77)
	at org.jruby.runtime.Block.call(Block.java:129)
	at org.jruby.RubyProc.call(RubyProc.java:299)
	at org.jruby.internal.runtime.methods.ProcMethod.call(ProcMethod.java:64)
	at org.jruby.ir.runtime.IRRuntimeHelpers.unresolvedSuper(IRRuntimeHelpers.java:1206)
	at blah.invokeSuper0:-unknown-super-target-(blah.rb:3)
	at blah.RUBY$method$warn$1(blah.rb:3)

This may be a bug in super or something wrong with the define_method logic for Method objects (like it's supposed to re-home it to the new anonymous module in the new warn logic.

The warn patch is here: https://github.com/rubygems/rubygems/blob/master/lib/rubygems/core_ext/kernel_warn.rb

This would block #5996 but I will commit the update with this patch removed.

This may not be a problem in general; it only seems to trigger the stack overflow within the ruby/test_exception.rb test shown below:

  1) Failure:
TestException#test_warning_warn_super [/home/travis/build/jruby/jruby/test/mri/ruby/test_exception.rb:1052]:

1. [2/2] Assertion for "stderr"
   | Expected /instance variable @a not initialized/ to match "Error: Your application used more stack memory than the safety cap of 2048K.\n" +
   | "Specify -J-Xss####k to increase it (#### = cap size in KB).\n" +
   | "Specify -w for full java.lang.StackOverflowError stack trace\n".
@headius headius added this to the JRuby 9.2.10.0 milestone Dec 11, 2019
headius added a commit to headius/jruby that referenced this issue Feb 12, 2020
It turns out MRI runs this way, which avoids running into the
stack overflow in jruby#5997. We are actually doing the fixed logic
correctly, but in the presence of that RubyGems warn patch, all
versions of MRI also fail.
@headius
Copy link
Member Author

headius commented Feb 14, 2020

Ok I figured this one out. It fails in CRuby as well.

The test in question was added after Ruby 2.5.7 and I don't believe it has made it into a 2.5 release. That bug and patch is here: https://bugs.ruby-lang.org/issues/14006

We already have this change in place as of JRuby 9.2.9.

The other discovery I made was that even with the patch, this test still fails if RubyGems' warn patch is loaded. CRuby runs those tests with --disable-gems which is why they passed it and we did not. I added this to the update_rubygems branch and the tests pass.

So this one is actually fixed.

@headius headius closed this as completed Feb 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant