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

regressed indy fixnum op with custom equality (==) #5938

Closed
kares opened this issue Oct 23, 2019 · 6 comments · Fixed by #5939
Closed

regressed indy fixnum op with custom equality (==) #5938

kares opened this issue Oct 23, 2019 · 6 comments · Fixed by #5939

Comments

@kares
Copy link
Member

@kares kares commented Oct 23, 2019

tracking a regression from elastic/logstash#11196 (comment)

... which boiled down to be a separate JRuby 9.2.8.0 issue (with -Xcompile.invokedynamic)

the regression was introduced with #4736 and relates to == comparison, sample reproducer:

class Primitive

  def _value
    @__value ||= 0
  end

  def ==(other)
    other == _value
  end

  # def <=>(other)
  #   _value <=> other
  # end

  # def eql?(other)
  #   # double dispatch
  #   other.eql?(snapshot)
  # end

end

class Klass; end

threads = []

3.times do |i|
  threads << Thread.start do

    begin

      header = Klass.new

      def header.version
        Primitive.new
      end

      header.version == 5
    rescue java.lang.Exception => ex
      puts("FAILED " + java.lang.Thread.currentThread.name + " - " + ex.toString)
      raise ex
    end

  end
end

threads.each &:join

p :done
Unhandled Java exception: java.lang.ClassCastException: org.jruby.gen.RubyObject1 cannot be cast to org.jruby.RubyFixnum
java.lang.ClassCastException: org.jruby.gen.RubyObject1 cannot be cast to org.jruby.RubyFixnum
            fixnum_op_equal at org/jruby/runtime/invokedynamic/MathLinker.java:234
        invokeWithArguments at java/lang/invoke/MethodHandle.java:627
             fixnumOperator at org/jruby/runtime/invokedynamic/MathLinker.java:168
  bug_bindata_reproducer.rb at bug_bindata_reproducer.rb:41
                 callDirect at org/jruby/runtime/CompiledIRBlockBody.java:136
                       call at org/jruby/runtime/IRBlockBody.java:77
                       call at org/jruby/runtime/Block.java:129
                       call at org/jruby/RubyProc.java:295
                       call at org/jruby/RubyProc.java:274
                       call at org/jruby/RubyProc.java:270
                        run at org/jruby/internal/runtime/RubyRunnable.java:105
                        run at java/lang/Thread.java:748
@kares kares added this to the JRuby 9.2.9.0 milestone Oct 23, 2019
@kares

This comment has been minimized.

Copy link
Member Author

@kares kares commented Oct 23, 2019

this seems to be problematic with indy since there's 2 bind paths happening for the some == 2 bind.
maybe we should try unifying so that the fixnum/float operation specialization work properly with indy invoke-site ... still not sure how much work that is - might be a bit naive here.

@kares

This comment has been minimized.

Copy link
Member Author

@kares kares commented Oct 23, 2019

the actual issue seems like a missing guard to NOT bind to a fixnum-op in case of the indy site (when the target isn't a Fixnum) a bit surprised this does not show up with tests ... there must be similar code.

@headius

This comment has been minimized.

Copy link
Member

@headius headius commented Oct 23, 2019

I am confused why the "isBuiltin" check doesn't kick it to the fail path. It's clearly not a built-in == implementation.

Agreed, though, it should at least be checking that the incoming self type actually is a RubyFixnum.

headius added a commit to headius/jruby that referenced this issue Oct 23, 2019
This primarily adds a hard RubyFixnum or RubyFloat type check on
self, to ensure we're not even attempting fast bindings for other
types. It also reduces overall binding overhead in that case by
moving direct indy logic out of the fail path and moves a couple
more adapted method handles to constants.

Fixes jruby#5938.
@kares kares closed this in #5939 Oct 24, 2019
kares added a commit that referenced this issue Oct 24, 2019
This primarily adds a hard RubyFixnum or RubyFloat type check on
self, to ensure we're not even attempting fast bindings for other
types. It also reduces overall binding overhead in that case by
moving direct indy logic out of the fail path and moves a couple
more adapted method handles to constants.

Fixes #5938.
@jsvd

This comment has been minimized.

Copy link

@jsvd jsvd commented Oct 25, 2019

hi @kares,
we're now seeing a different error, even with latest jruby snapshot (8b47c28):

[2019-10-25T09:52:43,686][ERROR][logstash.inputs.udp      ][main] Exception in inputworker {"exception"=>java.lang.ClassCastException: class org.jruby.gen.RubyObject4 cannot be cast to class org.jruby.RubyFixnum (org.jruby.gen.RubyObject4 is in unnamed module of loader org.jruby.util.OneShotClassLoader @3611153f; org.jruby.RubyFixnum is in unnamed module of loader 'app'), "backtrace"=>["org.jruby.runtime.invokedynamic.MathLinker.fixnum_op_div(MathLinker.java:233)", "java.base/java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:710)", "org.jruby.runtime.invokedynamic.MathLinker.fixnumOperator(MathLinker.java:171)", "private.tmp.logstash.vendor.bundle.jruby.$2_dot_5_dot_0.gems.logstash_minus_codec_minus_netflow_minus_4_dot_2_dot_1.lib.logstash.codecs.netflow.RUBY$block$decode_netflow5$2(/private/tmp/logstash/vendor/bundle/jruby/2.5.0/gems/logstash-codec-netflow-4.2.1/lib/logstash/codecs/netflow.rb:139)", "org.jruby.runtime.CompiledIRBlockBody.yieldDirect(CompiledIRBlockBody.java:146)", "org.jruby.runtime.BlockBody.yield(BlockBody.java:114)", "org.jruby.runtime.Block.yield(Block.java:170)", "org.jruby.ir.runtime.IRRuntimeHelpers.yield(IRRuntimeHelpers.java:477)", "org.jruby.ir.targets.YieldSite.yield(YieldSite.java:105)", "private.tmp.logstash.vendor.bundle.jruby.$2_dot_5_dot_0.gems.bindata_minus_2_dot_4_dot_4.lib.bindata.struct.RUBY$block$each_pair$1(/private/tmp/logstash/vendor/bundle/jruby/2.5.0/gems/bindata-2.4.4/lib/bindata/struct.rb:169)", "org.jruby.runtime.CompiledIRBlockBody.yieldDirect(CompiledIRBlockBody.java:146)", "org.jruby.runtime.BlockBody.yield(BlockBody.java:114)", "org.jruby.runtime.Block.yield(Block.java:170)", "org.jruby.RubyArray.each(RubyArray.java:1800)", "org.jruby.RubyArray$INVOKER$i$0$0$each.call(RubyArray$INVOKER$i$0$0$each.gen)", "org.jruby.internal.runtime.methods.JavaMethod$JavaMethodZeroBlock.call(JavaMethod.java:555)", "org.jruby.ir.targets.InvokeSite.invoke(InvokeSite.java:177)", "private.tmp.logstash.vendor.bundle.jruby.$2_dot_5_dot_0.gems.bindata_minus_2_dot_4_dot_4.lib.bindata.struct.RUBY$method$each_pair$0(/private/tmp/logstash/vendor/bundle/jruby/2.5.0/gems/bindata-2.4.4/lib/bindata/struct.rb:168)", "private.tmp.logstash.vendor.bundle.jruby.$2_dot_5_dot_0.gems.bindata_minus_2_dot_4_dot_4.lib.bindata.struct.RUBY$method$each_pair$0$__VARARGS__(/private/tmp/logstash/vendor/bundle/jruby/2.5.0/gems/bindata-2.4.4/lib/bindata/struct.rb)", "org.jruby.internal.runtime.methods.CompiledIRMethod.call(CompiledIRMethod.java:91)", "org.jruby.internal.runtime.methods.MixedModeIRMethod.call(MixedModeIRMethod.java:90)", "org.jruby.ir.targets.InvokeSite.invoke(InvokeSite.java:177)", "private.tmp.logstash.vendor.bundle.jruby.$2_dot_5_dot_0.gems.logstash_minus_codec_minus_netflow_minus_4_dot_2_dot_1.lib.logstash.codecs.netflow.RUBY$method$decode_netflow5$0(/private/tmp/logstash/vendor/bundle/jruby/2.5.0/gems/logstash-codec-netflow-4.2.1/lib/logstash/codecs/netflow.rb:130)", "private.tmp.logstash.vendor.bundle.jruby.$2_dot_5_dot_0.gems.logstash_minus_codec_minus_netflow_minus_4_dot_2_dot_1.lib.logstash.codecs.netflow.RUBY$method$decode_netflow5$0$__VARARGS__(/private/tmp/logstash/vendor/bundle/jruby/2.5.0/gems/logstash-codec-netflow-4.2.1/lib/logstash/codecs/netflow.rb)", "org.jruby.internal.runtime.methods.CompiledIRMethod.call(CompiledIRMethod.java:91)", "org.jruby.internal.runtime.methods.MixedModeIRMethod.call(MixedModeIRMethod.java:90)", "org.jruby.ir.targets.InvokeSite.invoke(InvokeSite.java:183)", "private.tmp.logstash.vendor.bundle.jruby.$2_dot_5_dot_0.gems.logstash_minus_codec_minus_netflow_minus_4_dot_2_dot_1.lib.logstash.codecs.netflow.RUBY$block$decode$1(/private/tmp/logstash/vendor/bundle/jruby/2.5.0/gems/logstash-codec-netflow-4.2.1/lib/logstash/codecs/netflow.rb:88)", "org.jruby.runtime.CompiledIRBlockBody.yieldDirect(CompiledIRBlockBody.java:146)", "org.jruby.runtime.BlockBody.yield(BlockBody.java:114)", "org.jruby.runtime.Block.yield(Block.java:170)", "org.jruby.ir.runtime.IRRuntimeHelpers.yield(IRRuntimeHelpers.java:477)", "org.jruby.ir.targets.YieldSite.yield(YieldSite.java:105)", "private.tmp.logstash.vendor.bundle.jruby.$2_dot_5_dot_0.gems.bindata_minus_2_dot_4_dot_4.lib.bindata.array.RUBY$block$each$1(/private/tmp/logstash/vendor/bundle/jruby/2.5.0/gems/bindata-2.4.4/lib/bindata/array.rb:208)", "org.jruby.runtime.CompiledIRBlockBody.yieldDirect(CompiledIRBlockBody.java:146)", "org.jruby.runtime.BlockBody.yield(BlockBody.java:114)", "org.jruby.runtime.Block.yield(Block.java:170)", "org.jruby.RubyArray.each(RubyArray.java:1800)", "org.jruby.RubyArray$INVOKER$i$0$0$each.call(RubyArray$INVOKER$i$0$0$each.gen)", "org.jruby.internal.runtime.methods.JavaMethod$JavaMethodZeroBlock.call(JavaMethod.java:555)", "org.jruby.ir.targets.InvokeSite.invoke(InvokeSite.java:177)", "private.tmp.logstash.vendor.bundle.jruby.$2_dot_5_dot_0.gems.bindata_minus_2_dot_4_dot_4.lib.bindata.array.RUBY$method$each$0(/private/tmp/logstash/vendor/bundle/jruby/2.5.0/gems/bindata-2.4.4/lib/bindata/array.rb:208)", "private.tmp.logstash.vendor.bundle.jruby.$2_dot_5_dot_0.gems.bindata_minus_2_dot_4_dot_4.lib.bindata.array.RUBY$method$each$0$__VARARGS__(/private/tmp/logstash/vendor/bundle/jruby/2.5.0/gems/bindata-2.4.4/lib/bindata/array.rb)", "org.jruby.internal.runtime.methods.CompiledIRMethod.call(CompiledIRMethod.java:91)", "org.jruby.internal.runtime.methods.MixedModeIRMethod.call(MixedModeIRMethod.java:90)", "org.jruby.ir.targets.InvokeSite.invoke(InvokeSite.java:177)", "private.tmp.logstash.vendor.bundle.jruby.$2_dot_5_dot_0.gems.logstash_minus_codec_minus_netflow_minus_4_dot_2_dot_1.lib.logstash.codecs.netflow.RUBY$method$decode$0(/private/tmp/logstash/vendor/bundle/jruby/2.5.0/gems/logstash-codec-netflow-4.2.1/lib/logstash/codecs/netflow.rb:87)", "org.jruby.internal.runtime.methods.CompiledIRMethod.call(CompiledIRMethod.java:91)", "org.jruby.internal.runtime.methods.MixedModeIRMethod.call(MixedModeIRMethod.java:90)", "org.jruby.ir.targets.InvokeSite.invoke(InvokeSite.java:177)", "private.tmp.logstash.logstash_minus_core.lib.logstash.codecs.delegator.RUBY$block$decode$1(/private/tmp/logstash/logstash-core/lib/logstash/codecs/delegator.rb:45)", "org.jruby.runtime.CompiledIRBlockBody.callDirect(CompiledIRBlockBody.java:136)", "org.jruby.runtime.IRBlockBody.call(IRBlockBody.java:77)", "org.jruby.runtime.IRBlockBody.call(IRBlockBody.java:51)", "org.jruby.runtime.Block.call(Block.java:133)", "org.logstash.instrument.metrics.MetricExt.doTime(MetricExt.java:140)", "org.logstash.instrument.metrics.AbstractSimpleMetricExt.time(AbstractSimpleMetricExt.java:45)", "org.logstash.instrument.metrics.NamespacedMetricExt.doTime(NamespacedMetricExt.java:87)", "org.logstash.instrument.metrics.AbstractNamespacedMetricExt.time(AbstractNamespacedMetricExt.java:44)", "private.tmp.logstash.logstash_minus_core.lib.logstash.codecs.delegator.RUBY$method$decode$0(/private/tmp/logstash/logstash-core/lib/logstash/codecs/delegator.rb:44)", "private.tmp.logstash.logstash_minus_core.lib.logstash.codecs.delegator.RUBY$method$decode$0$__VARARGS__(/private/tmp/logstash/logstash-core/lib/logstash/codecs/delegator.rb)", "org.jruby.internal.runtime.methods.CompiledIRMethod.call(CompiledIRMethod.java:91)", "org.jruby.internal.runtime.methods.MixedModeIRMethod.call(MixedModeIRMethod.java:90)", "org.jruby.ir.targets.InvokeSite.invoke(InvokeSite.java:177)", "private.tmp.logstash.vendor.bundle.jruby.$2_dot_5_dot_0.gems.logstash_minus_input_minus_udp_minus_3_dot_3_dot_4.lib.logstash.inputs.udp.RUBY$method$inputworker$0(/private/tmp/logstash/vendor/bundle/jruby/2.5.0/gems/logstash-input-udp-3.3.4/lib/logstash/inputs/udp.rb:151)", "private.tmp.logstash.vendor.bundle.jruby.$2_dot_5_dot_0.gems.logstash_minus_input_minus_udp_minus_3_dot_3_dot_4.lib.logstash.inputs.udp.RUBY$method$inputworker$0$__VARARGS__(/private/tmp/logstash/vendor/bundle/jruby/2.5.0/gems/logstash-input-udp-3.3.4/lib/logstash/inputs/udp.rb)", "org.jruby.internal.runtime.methods.CompiledIRMethod.call(CompiledIRMethod.java:91)", "org.jruby.internal.runtime.methods.MixedModeIRMethod.call(MixedModeIRMethod.java:90)", "org.jruby.ir.targets.InvokeSite.invoke(InvokeSite.java:183)", "private.tmp.logstash.vendor.bundle.jruby.$2_dot_5_dot_0.gems.logstash_minus_input_minus_udp_minus_3_dot_3_dot_4.lib.logstash.inputs.udp.RUBY$block$run$2(/private/tmp/logstash/vendor/bundle/jruby/2.5.0/gems/logstash-input-udp-3.3.4/lib/logstash/inputs/udp.rb:63)", "org.jruby.runtime.CompiledIRBlockBody.callDirect(CompiledIRBlockBody.java:136)", "org.jruby.runtime.IRBlockBody.call(IRBlockBody.java:77)", "org.jruby.runtime.Block.call(Block.java:129)", "org.jruby.RubyProc.call(RubyProc.java:295)", "org.jruby.RubyProc.call(RubyProc.java:274)", "org.jruby.RubyProc.call(RubyProc.java:270)", "org.jruby.internal.runtime.RubyRunnable.run(RubyRunnable.java:105)", "java.base/java.lang.Thread.run(Thread.java:834)"]}

steps are described here: elastic/logstash#11196 (comment)

@kares

This comment has been minimized.

Copy link
Member Author

@kares kares commented Oct 25, 2019

oh good catch - think I know the cause, thanks @jsvd ... will sort this one out as I get back (later today).

was in the process of adding tests for the original issue got a bit lost there ... reproducing in a test env.

@kares

This comment has been minimized.

Copy link
Member Author

@kares kares commented Oct 25, 2019

we simply introduced a NPE: 4122fcb ... fixed now with 321bf9c
still need tests - to be added later

kares added a commit to kares/jruby that referenced this issue Oct 26, 2019
kares added a commit to kares/jruby that referenced this issue Oct 26, 2019
likely (potentially) happening along with jrubyGH-5938
kares added a commit to kares/jruby that referenced this issue Oct 26, 2019
kares added a commit to kares/jruby that referenced this issue Oct 26, 2019
likely (potentially) happening along with jrubyGH-5938
kares added a commit that referenced this issue Oct 26, 2019
likely (potentially) happening along with GH-5938
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants
You can’t perform that action at this time.