Fixes #6563. Rails 6.1.2.x dynamic finders cause Java::JavaLang::ClassCastException#6571
Merged
enebo merged 6 commits intojruby:jruby-9.2from Feb 23, 2021
Merged
Fixes #6563. Rails 6.1.2.x dynamic finders cause Java::JavaLang::ClassCastException#6571enebo merged 6 commits intojruby:jruby-9.2from
enebo merged 6 commits intojruby:jruby-9.2from
Conversation
…er in module bodies?
This was never part of Ruby and a misunderstanding that Ruby does
not support this.
The feature was never hit because people tend to target MRI first
so we never had people doing this. Recent Rails find_by logic exposed
a bug in this feature which would CCE.
Removing this fixes the CCE.
The second part of this commit is raising the RuntimeError pointing
out zsuper is not supported in blocks.
This uses a faulty heuristic that can be changed if we realize someone
somehow is doing something which breaks the heuristic. We will raise
if we see a closure which is attached to a method call which is
define_method in the presence of existing in a method scope. This means
if someone happens to have a zsuper in another method called define_method
in a def then we will not properly invoke the zsuper.
This also exposes an issue I think with our bytecode generation of
RaiseException which can be seen by running:
```ruby
def test_define_method
a = Object.new
lambda {
Class.new {
define_method(:a) {super}
}.new.a
}.call
end
test_define_method
```
So a followup commit will come to address that and possibly also expanding
the above heuristic to define_singleton_method.
Interpreter was unwrapping and getting an exception object to throw with throwExceptionIstr. The JIT was only working with actual exceptions. This commit aligns the interpreter with the JIT and the only two uses which would not work just become dynamic calls to Kernel#raise. This is fine because it does not complicate JIT by making throwinstr more complicated and the dynamic calls only occur extremely extremely rarely to raise errors. The second fix is that define_method can sometimes convert itself from being a closure to being a method. This conversion did not care whether zsuper was present or not. We will now mark zsuper in the closure scope and not perform this optimization in that case.
Reverted this logic. The define_method and flip flop are stil dyncalls but there is clearly more going on here. Interesting too since the JIT would crash if it received an IRubyObject instead of a Throwable. It must handle some of this differently in the JIT.
headius
approved these changes
Feb 23, 2021
| return zsuperResult; | ||
| } | ||
| }; | ||
| private boolean isMethodDefine() { |
Member
Author
There was a problem hiding this comment.
yeah I yoda named that for no reason. I will change that.
| runtime.newSymbol("raise"), | ||
| kernel, | ||
| new Operand[] { exception }, | ||
| null)); |
Member
There was a problem hiding this comment.
You could just call this as Kernel.raise WhateverException, "message" rather than doing the call to new.
Member
Author
There was a problem hiding this comment.
sure. I can make this change...less generated instrs + bytecode.
Member
Author
|
Spurious failure on travis on some MT test writing IO. Unrelated to this. |
jsvd
added a commit
to logstash-plugins/logstash-output-s3
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-filter-elasticsearch
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-filter-fingerprint
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-filter-grok
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-filter-prune
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-filter-ruby
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-filter-xml
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-input-beats
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-input-elasticsearch
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-input-file
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-input-http
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-input-jms
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-input-jdbc
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-input-lumberjack
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-input-rabbitmq
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-input-s3
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-input-snmp
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-input-sqs
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-input-twitter
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-integration-elasticsearch
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-integration-jdbc
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-integration-kafka
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-integration-rabbitmq
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-mixin-rabbitmq_connection
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-output-elasticsearch
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-output-hipchat
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-output-http
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-output-loggly
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-output-sqs
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to logstash-plugins/logstash-output-tcp
that referenced
this pull request
Mar 3, 2021
jsvd
added a commit
to jsvd/logstash
that referenced
this pull request
Mar 3, 2021
fix define_method+super calls due to jruby/jruby#6571
kares
added a commit
to kares/logstash-output-elasticsearch
that referenced
this pull request
Mar 4, 2021
* master: [tests] change super to super() - jruby/jruby#6571
jsvd
added a commit
to elastic/logstash
that referenced
this pull request
Mar 4, 2021
fix define_method+super calls due to jruby/jruby#6571
jsvd
added a commit
to jsvd/logstash
that referenced
this pull request
Mar 4, 2021
fix define_method+super calls due to jruby/jruby#6571 (cherry picked from commit 9643a33)
jsvd
added a commit
to elastic/logstash
that referenced
this pull request
Mar 4, 2021
fix define_method+super calls due to jruby/jruby#6571 (cherry picked from commit 9643a33)
kaisecheng
pushed a commit
to kaisecheng/logstash-input-http
that referenced
this pull request
Jun 8, 2021
kaisecheng
pushed a commit
to kaisecheng/logstash-input-http
that referenced
this pull request
Jun 8, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #6563
90% of the changes here are for JRuby to dynamically raise an exception if we try and use zsuper from within a define_method block.
The important part is instead of building up a list of argument operands at each level so we can support zsuper we just delete all that. So removing the logic we should not have been doing in the first place means we do not hit a bug in that code in the first place.