backport a JRuby 9.1.16.0 stdlib resolv.rb patch in installed 9.1.13.0 #9750
Conversation
| # JRuby versions prior to 9.1.16.0 have a bug which crashes IP address | ||
| # the resolver after 64k unique IP addresses resolutions. | ||
|
|
||
| version = JRUBY_VERSION.split(".").map(&:to_i) |
There was a problem hiding this comment.
let's do the same here as in logstash-plugins/logstash-filter-dns#45
|
@jsvd let me know if you are ok with the changes. |
|
|
||
| # make sure we abort if a known correct JRuby version is installed | ||
| # to avoid having an unnecessary legacy patch being applied in the future. | ||
| raise("Unnecessary patch on resolv.rb for JRuby version 9.1.16+") if Gem::Version.new(JRUBY_VERSION) >= Gem::Version.new("9.1.16.0") |
There was a problem hiding this comment.
Why not just skip the patch like we do in https://github.com/logstash-plugins/logstash-filter-dns/pull/45/files?
There was a problem hiding this comment.
the idea is that if we hit that condition and raise, tests will break and will be an indication that we can remove that patch altogether. I don't see the point of keeping that patch code around if it is not required. I rather have the tests fail than forget about the useless patch code in the future.
|
|
||
| # The code below is copied from JRuby 9.1.16.0 resolv.rb: | ||
| # https://github.com/jruby/jruby/blob/9.1.16.0/lib/ruby/stdlib/resolv.rb#L775-L784 | ||
|
|
There was a problem hiding this comment.
Include copyright and license from https://github.com/jruby/jruby/blob/9.1.16.0/COPYING and explicitly indicate that we are redistribution under EPL:
# Copyright (c) 2007-2018 The JRuby project.
# Redistributed under the terms of the Eclipse Public License.There was a problem hiding this comment.
sure. but this is an interesting case of including JRuby code for the purpose of patching JRuby itself using metaprogramming (not really for executing that code within LS) for which we already implicitly respect its copyright+license since we already include it as source code and the license terms have not changed between .13 and .16.
…JRuby version use Gem::Version to compare versions and improve comments include JRuby license info
|
rebasing/squashing and merging. |
The installed JRuby version 9.1.13.0 has a bug in the stdlib resolv.rb that has been fixed in 9.1.16.0
This bug result in the resolver hanging after 64k unique IP resolutions. This problem has surfaced in the dns filter, documented in logstash-plugins/logstash-filter-dns/issues/40
I think we should apply this patch in core to make sure any other resolver usage does not result in hanging logstash moving forward until JRuby version 9.1.16.0 or newer is installed in core.
I will also created this monkey patch in the dns filter (logstash-plugins/logstash-filter-dns/issues/45) to be able to release quickly a fix for that specific problem in the filter. I do not see any potential problem in having this patch in the two places.