Kernel.load(file, true) produces RuntimeException: Should not get here! #3180

dekellum · 2015-07-24T16:06:40Z

Jruby, hasn't had support for the wrap=true option of Kernel::load. While previously (1.6.8, 1.7.0, 1.7.16, 1.7.21) it would not honor this option, it now (9.0.0.0) produces a runtime exception java.lang.RuntimeException: Should not get here! scopeType is SCRIPT_BODY if the wrap option is set.

There was previously a JRUBY-4339 on codehaus:

http://ruby.11.x6.nabble.com/jira-Created-JRUBY-4339-Kernel-load-with-wrap-true-does-not-protect-the-global-namespace-of-calling-m-td3480038.html

The jruby 1.7.0 release notes suggest this was fixed?

http://jruby.org/2012/10/22/jruby-1-7-0.html

...but my testing doesn't agree. Here is a minimal test setup:

https://github.com/dekellum/test-load-wrap

And here is output from MRI (passing); jruby 1.7.0, 1.7.16, 1.7.21 (shows not honoring wrap); and jruby 9.0.0.0 (new RuntimeException raised). For prior 9.x pre-releases I believe #3126 was preventing me from getting this far to see this issue.

% ruby -v loader.rb 
ruby 2.2.2p95 (2015-04-13 revision 50295) [x86_64-linux]
Top level loader method called from loadee

% jruby -v loader.rb 
jruby 1.7.0 (1.9.3p203) 2012-10-22 ff1ebbe on OpenJDK 64-Bit Server VM 1.8.0_51-b16 +indy [linux-amd64]
Top level loader method called from loadee
RuntimeError: ERROR: Given load() with wrap=true, loadee's foo shouldn't be defined
  (root) at loader.rb:8

% jruby -v loader.rb 
jruby 1.7.16 (1.9.3p392) 2014-09-25 575b395 on OpenJDK 64-Bit Server VM 1.8.0_51-b16 +jit [linux-amd64]
Top level loader method called from loadee
RuntimeError: ERROR: Given load() with wrap=true, loadee's foo shouldn't be defined
  (root) at loader.rb:8

% jruby -v loader.rb 
jruby 1.7.21 (1.9.3p551) 2015-07-07 a741a82 on OpenJDK 64-Bit Server VM 1.8.0_51-b16 +jit [linux-amd64]
Top level loader method called from loadee
RuntimeError: ERROR: Given load() with wrap=true, loadee's foo shouldn't be defined
  (root) at loader.rb:8

% jruby -v loader.rb 
jruby 9.0.0.0 (2.2.2) 2015-07-21 e10ec96 OpenJDK 64-Bit Server VM 25.51-b03 on 1.8.0_51-b16 +jit [linux-amd64]
Top level loader method called from loadee
IRRuntimeHelpers.java:789:in `findInstanceMethodContainer': java.lang.RuntimeException: Should not get here! scopeType is SCRIPT_BODY
    from IRRuntimeHelpers.java:1259:in `defInterpretedInstanceMethod'
    from DefineInstanceMethodInstr.java:56:in `interpret'
    from StartupInterpreterEngine.java:183:in `processOtherOp'
    from StartupInterpreterEngine.java:107:in `interpret'
    from Interpreter.java:116:in `INTERPRET_ROOT'
    from Interpreter.java:103:in `execute'
    from Interpreter.java:32:in `execute'
    from IRTranslator.java:42:in `execute'
    from Ruby.java:837:in `runInterpreter'
    from Ruby.java:2901:in `loadFile'
    from LibrarySearcher.java:245:in `load'
    from LibrarySearcher.java:35:in `load'
    from LoadService.java:335:in `load'
    from RubyKernel.java:966:in `loadCommon'
    from RubyKernel.java:958:in `load19'
    from RubyKernel$INVOKER$s$0$1$load19.gen:-1:in `call'
    from DynamicMethod.java:213:in `call'
    from DynamicMethod.java:209:in `call'
    from CachingCallSite.java:333:in `cacheAndCall'
    from CachingCallSite.java:195:in `call'
    from loader.rb:-1:in `invokeOther7:load'
    from loader.rb:5:in `RUBY$script'
    from MethodHandle.java:625:in `invokeWithArguments'
    from Compiler.java:111:in `load'
    from Ruby.java:821:in `runScript'
    from Ruby.java:813:in `runScript'
    from Ruby.java:751:in `runNormally'
    from Ruby.java:573:in `runFromMain'
    from Main.java:403:in `doRunFromMain'
    from Main.java:298:in `internalRun'
    from Main.java:225:in `run'
    from Main.java:197:in `main'

The text was updated successfully, but these errors were encountered:

enebo · 2015-07-24T16:26:18Z

Yeah. Perhaps at some point we regressed on 1.7? At some point we had some large internal changes to LoadService. Crash in 9k is an IR issue but could be directly supported to us not properly supporting wrap.

dekellum · 2015-07-24T17:19:56Z

That's what I had imagined, but see above: my test also fails on jruby 1.7.0.

In latest commit test-load-wrap I improved my test a bit. Same results.

enebo · 2015-07-24T18:31:55Z

@dekellum haha...very odd.

headius · 2016-01-15T15:14:01Z

Current results on 1.7 branch:

$ jruby loader.rb
Top level loader method called from loadee
RuntimeError: ERROR: With wrap=true, foo should't be defined at top level
     foo at /Users/headius/projects/jruby-1.7/test-load-wrap/loadee.rb:4
  (root) at loader.rb:8

I'll have a look.

headius · 2016-01-15T16:49:58Z

I think I have a patch that fixes wrapping.

Part of work for 1.7 load(..., true) for #3180.

headius · 2016-01-15T17:05:23Z

Ok, I've patched this in 1.7 and it seems ok. Both interpreter and AOT scenarios properly isolate definitions when wrap = true.

I'm looking into 9k now, and the error is a bit different:

$ jruby -I. loader.rb 
Top level loader method called from loadee
Unhandled Java exception: java.lang.ClassCastException: org.jruby.RubyObject cannot be cast to org.jruby.RubyModule
java.lang.ClassCastException: org.jruby.RubyObject cannot be cast to org.jruby.RubyModule
   findInstanceMethodContainer at org/jruby/ir/runtime/IRRuntimeHelpers.java:819
  defInterpretedInstanceMethod at org/jruby/ir/runtime/IRRuntimeHelpers.java:1300
                     interpret at org/jruby/ir/instructions/DefineInstanceMethodInstr.java:56
                processOtherOp at org/jruby/ir/interpreter/StartupInterpreterEngine.java:191
                     interpret at org/jruby/ir/interpreter/StartupInterpreterEngine.java:115

headius · 2016-01-15T17:17:27Z

Ok, looks like a hack I put in place while trying to get block call protocol working in JIT was left in, and causes the above error. Removing that hack gets me back to the "should not get here" error. I'll investigate from there.

This is part of work for #3180. This appears to fix issues with load(..., true) not actually wrapping method definitions in an anonymous module.

headius · 2016-01-15T18:21:30Z

Ok, I think this is working properly on master too now. There were specs on master we did not pass that pass now, so I'm going to call this fixed and close it.

@dekellum If you get a chance, verify jruby-1_7 branch and master builds (or grab tomorrow from http://ci.jruby.org) and let us know that things are fixed for you!

Since it fails hard (raises) on these versions. See jruby/jruby#3180

See jruby/jruby#3180

dekellum · 2016-01-18T14:27:25Z

I can confirm that the 1.7.24-SNAPSHOT (2016-01-18T05:10:56.000Z) is now working fine. The 9.0.5.0-SNAPSHOT (2016-01-18T05:38:12.000Z) is working for the simple test provided above but is failing in a new way in my more complex application (syncwrap, a method included at top-level is no longer found.) I'll try to expand the above simple test to reproduce this, and report back.

This is more analogous to the usage in the Syncwrap project. cc: jruby/jruby#3180

dekellum · 2016-01-18T23:38:03Z

I updated the test here: dekellum/test-load-wrap@44d50f9 to include this case that still fails on jruby 9.0.5.0-SNAPSHOT, as shown in the last case below. As I can't reopen this issue, I'll wait for a response before opening any new issue. Thanks.

% ruby -v loader.rb
ruby 2.2.4p230 (2015-12-16 revision 53155) [x86_64-linux]
Top level loader method called from loadee
The load(...,true) wrap option works

% jruby -v ./loader.rb 
jruby 1.7.24-SNAPSHOT (1.9.3p551) 2016-01-18 e7e3815 on OpenJDK 64-Bit Server VM 1.8.0_66-b17 +jit [linux-amd64]
Top level loader method called from loadee
The load(...,true) wrap option works

% jruby -v ./loader.rb 
jruby 9.0.5.0-SNAPSHOT (2.2.3) 2016-01-18 7109a23 OpenJDK 64-Bit Server VM 25.66-b17 on 1.8.0_66-b17 +jit [linux-amd64]
NameError: undefined local variable or method `bar' for main:Object
  <top> at /home/david/src/test-load-wrap/loadee.rb:1
   load at org/jruby/RubyKernel.java:955
  <top> at ./loader.rb:10

* jruby-1_7: (27 commits) Update jnr-posix to finally fix 32 bit JVM crasher on windows stat pend marshaling behavior - not working for Java non-member classes (GH-3526) local (non-member) classes should have correct name just like anonymous classes DRY out getClassFromPath usage in UnmarshalStream (all delegating to one method) review runtime's getClassFromPath hide unmarshalObject methods that use private inner type as a parameter Update for newer jnr-posix for windows 32 bit JVM stat workaround Remove remaining `should` uses from compiler specs. Don't deoptimize while loops just because of a block arg. Always assign masgn destructured args in block as block-local. Move parser scripts to tool. Add case for breakage fixed in b93fbb6e431. Check for empty identifiers and treat them as false. Same fix for AOT as for interp for #3180. Also use the anonymous wrap class for pushRubyClass, for defs. More robust, less breaky fix to omit bad class/package elements. Revert "Use methods instead of procs for converters." Fixes #3550. Warning "io/console not supported; tty will not be manipulated" occurs again on 1.7.23 updating to jcodings messes with this obscure char no longer being considered a space. Update to latest jcodings ...

cc: jruby/jruby#3180 jruby/jruby#3609

headius · 2016-02-14T12:45:43Z

Oops, catching up...I guess we did fix this in response to #3609.

enebo added core JRuby 1.7.x JRuby 9000 labels Jul 24, 2015

enebo added this to the JRuby 1.7.22 milestone Jul 24, 2015

enebo modified the milestones: JRuby 1.7.22, JRuby 1.7.23 Aug 20, 2015

enebo modified the milestones: JRuby 1.7.23, JRuby 1.7.24 Nov 24, 2015

headius added a commit that referenced this issue Jan 15, 2016

Also use the anonymous wrap class for pushRubyClass, for defs.

73daf0a

Part of work for 1.7 load(..., true) for #3180.

headius added a commit that referenced this issue Jan 15, 2016

Same fix for AOT as for interp for #3180.

2e2647e

headius added a commit that referenced this issue Jan 15, 2016

Remove this hack in prep for investigating #3180.

940c5aa

headius added a commit that referenced this issue Jan 15, 2016

Also get self's class for method defs in script body.

eb69173

This is part of work for #3180. This appears to fix issues with load(..., true) not actually wrapping method definitions in an anonymous module.

headius closed this as completed Jan 15, 2016

dekellum added a commit to dekellum/syncwrap that referenced this issue Jan 18, 2016

Avoid attempting to wrap load of sync file on JRuby 9.0.[0-4]

06cffc0

Since it fails hard (raises) on these versions. See jruby/jruby#3180

dekellum added a commit to dekellum/syncwrap that referenced this issue Jan 18, 2016

Skip space main (wrapping) test on specific JRuby versions

3966456

See jruby/jruby#3180

dekellum added a commit to dekellum/test-load-wrap that referenced this issue Jan 18, 2016

Add test of top-level extended bar from loadee.

44d50f9

This is more analogous to the usage in the Syncwrap project. cc: jruby/jruby#3180

dekellum mentioned this issue Jan 20, 2016

Incompatible method lookup under Kernel.load(file,true), aka wrap, on 9.x #3609

Closed

dekellum added a commit to dekellum/syncwrap that referenced this issue Jan 20, 2016

Avoid attempting to wrap load of sync file on JRuby 9.x (for now)

a072575

cc: jruby/jruby#3180 jruby/jruby#3609

dekellum added a commit to dekellum/syncwrap that referenced this issue Jan 20, 2016

Skip space main (wrapping) test on 9.0.x (for now)

758ac6a

cc: jruby/jruby#3180 jruby/jruby#3609

headius reopened this Feb 14, 2016

headius closed this as completed Feb 14, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GitHub Sponsors

Kernel.load(file, true) produces RuntimeException: Should not get here! #3180

Kernel.load(file, true) produces RuntimeException: Should not get here! #3180

dekellum commented Jul 24, 2015

enebo commented Jul 24, 2015

dekellum commented Jul 24, 2015

enebo commented Jul 24, 2015

headius commented Jan 15, 2016

headius commented Jan 15, 2016

headius commented Jan 15, 2016

headius commented Jan 15, 2016

headius commented Jan 15, 2016

dekellum commented Jan 18, 2016

dekellum commented Jan 18, 2016

headius commented Feb 14, 2016

Kernel.load(file, true) produces RuntimeException: Should not get here! #3180

Kernel.load(file, true) produces RuntimeException: Should not get here! #3180

Comments

dekellum commented Jul 24, 2015

enebo commented Jul 24, 2015

dekellum commented Jul 24, 2015

enebo commented Jul 24, 2015

headius commented Jan 15, 2016

headius commented Jan 15, 2016

headius commented Jan 15, 2016

headius commented Jan 15, 2016

headius commented Jan 15, 2016

dekellum commented Jan 18, 2016

dekellum commented Jan 18, 2016

headius commented Feb 14, 2016