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.

Sign up for GitHub

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

Jump to bottom

NullPointerException when using refinement including multiple modules and calling super #4237

Closed
jeremyevans opened this issue Oct 19, 2016 · 7 comments
Closed

NullPointerException when using refinement including multiple modules and calling super #4237

jeremyevans opened this issue Oct 19, 2016 · 7 comments
Milestone
JRuby 9.2.7.0

Comments

@jeremyevans
Copy link
Contributor

Environment

Provide at least:
jruby 9.1.5.0 (2.3.1) 2016-09-07 036ce39 OpenJDK 64-Bit Server VM 25.72-b15 on 1.8.0_72-b15 +jit [OpenBSD-x86_64]
OpenBSD foo 6.0 GENERIC.MP#0 amd64

Also tested with 9.1.1.0 on Windows, same issue.

Expected Behavior

module R1
  def *(v)
    [v, v]
  end
end

module R2
  def *(v)
    [self, super]
  end
end

module P
  refine Symbol do
    include R1
    include R2
  end
end

using P
p(:a * 1)

On ruby, prints [:a, [1, 1]]

Actual Behavior

On jruby:

Unhandled Java exception: java.lang.NullPointerException
java.lang.NullPointerException: null
   unresolvedSuper at org/jruby/ir/runtime/IRRuntimeHelpers.java:1037
         interpret at org/jruby/ir/instructions/UnresolvedSuperInstr.java:82
       processCall at org/jruby/ir/interpreter/InterpreterEngine.java:356
         interpret at org/jruby/ir/interpreter/StartupInterpreterEngine.java:73
         interpret at org/jruby/ir/interpreter/InterpreterEngine.java:84
  INTERPRET_METHOD at org/jruby/internal/runtime/methods/MixedModeIRMethod.java:179
              call at org/jruby/internal/runtime/methods/MixedModeIRMethod.java:165
              call at org/jruby/internal/runtime/methods/DynamicMethod.java:197
              call at org/jruby/runtime/callsite/RefinedCachingCallSite.java:73
              call at org/jruby/runtime/callsite/CachingCallSite.java:55
       processCall at org/jruby/ir/interpreter/InterpreterEngine.java:300
         interpret at org/jruby/ir/interpreter/StartupInterpreterEngine.java:73
    INTERPRET_ROOT at org/jruby/ir/interpreter/Interpreter.java:112
           execute at org/jruby/ir/interpreter/Interpreter.java:99
           execute at org/jruby/ir/interpreter/Interpreter.java:35
           execute at org/jruby/ir/IRTranslator.java:42
    runInterpreter at org/jruby/Ruby.java:869
    runInterpreter at org/jruby/Ruby.java:874
       runNormally at org/jruby/Ruby.java:754
       runNormally at org/jruby/Ruby.java:767
       runFromMain at org/jruby/Ruby.java:580
     doRunFromMain at org/jruby/Main.java:425
       internalRun at org/jruby/Main.java:313
               run at org/jruby/Main.java:242
              main at org/jruby/Main.java:204
@headius headius mentioned this issue Apr 27, 2018
Closed
@headius
Copy link
Member

headius commented Jan 31, 2019

@jeremyevans Hey, I have been looking at this and determined it's more of a problem with how we handle super inside a module than refinements specifically. For the moment I do not have a fix. Can you show me where you're using this pattern?

@headius headius mentioned this issue Jan 31, 2019
Closed
@jeremyevans
Copy link
Contributor Author

Here are the Sequel specs that fail with NullPointerException on JRuby if the guards are removed from the top of the file:

https://github.com/jeremyevans/sequel/blob/master/spec/extensions/core_refinements_spec.rb#L29
https://github.com/jeremyevans/sequel/blob/master/spec/extensions/core_refinements_spec.rb#L345

There are some other refinement related failures on JRuby that I added guards for at the same time, but may be a different refinement issue. All 4 specs in this block still fail on JRuby 9.2.5.0 if the guard is removed:

https://github.com/jeremyevans/sequel/blob/master/spec/extensions/s_spec.rb#L32

Here's the related commit to add the guards:

jeremyevans/sequel@4a334b1

@jeremyevans
Copy link
Contributor Author

I probably should have added links to the refinements themselves:

https://github.com/jeremyevans/sequel/blob/master/lib/sequel/extensions/core_refinements.rb
https://github.com/jeremyevans/sequel/blob/master/lib/sequel/extensions/s.rb

@headius
Copy link
Member

headius commented Jan 31, 2019

@jeremyevans Ok thank you. This will likely be fixed separate from the refinement work, since it's a symptom of the larger problem described in #5585, but we will prioritize it higher since it does get used in real-world code.

@headius
Copy link
Member

headius commented Feb 26, 2019

This does not appear to be fixed by #5604 and #5627 but I will try to fix it with follow-up refinement tweaks.

@headius headius added this to the JRuby 9.2.7.0 milestone Feb 26, 2019
@headius
Copy link
Member

headius commented Mar 7, 2019

This is still not fixed on master, but it's not a bug in refinements.

Calls of the form a + n where the n is a literal Fixnum or Float currently get compiled differently, to avoid the need for a numeric box (which can confuse some JITs when they try to eliminate other boxes). However the logic that selects call site logic in the JIT does not properly select a refined call site for these cases. In addition, the logic for a refined call site does not override this logic, so it would skip the refined lookup anyway.

Running it without JIT works fine:

[] ~/projects/jruby $ jruby -X-C blah.rb
[:a, [1, 1]]

This will be fixed before 9.2.7 since behavior differs between interpreter and JIT.

@headius
Copy link
Member

headius commented Mar 7, 2019

Oh FWIW it doesn't NPE now.

[] ~/projects/jruby $ jruby blah.rb
NoMethodError: undefined method `*' for :a:Symbol
  <main> at blah.rb:21

@headius headius closed this as completed in 2122e9f Apr 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
JRuby 9.2.7.0
Development

No branches or pull requests
2 participants
@jeremyevans @headius