From f304deff219f3a214f301ea978143a682f840496 Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Thu, 23 Feb 2023 15:47:00 -0600 Subject: [PATCH 01/31] Try and appease the eval+binding gods When we eval with binding we will construct a new binding that contains the original binding and that original binding will construct a new evalDynamicScope. If I do something like: ```ruby p TOPLEVEL_BINDING.local_variable_defined? :c p TOPLEVEL_BINDING.local_variable_defined? :_xxx_var_ b =TOPLEVEL_BINDING.dup p b.local_variable_defined? :c p b.local_variable_defined? :_xxx_var_ ``` I should still see _xxx_var_ after the dup. This will make that do that but two different RubyBinding instances will end up with the exact same backing Binding and I think this is wrong. Landing anyways to see if anything breaks. --- core/src/main/java/org/jruby/RubyBinding.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/jruby/RubyBinding.java b/core/src/main/java/org/jruby/RubyBinding.java index 76987c0c62e..778f304d025 100644 --- a/core/src/main/java/org/jruby/RubyBinding.java +++ b/core/src/main/java/org/jruby/RubyBinding.java @@ -104,7 +104,12 @@ public IRubyObject initialize(ThreadContext context) { return this; } - + + @JRubyMethod + public IRubyObject dup(ThreadContext context) { + return newBinding(context.runtime, binding); + } + @JRubyMethod(name = "initialize_copy", required = 1, visibility = Visibility.PRIVATE) @Override public IRubyObject initialize_copy(IRubyObject other) { From fa3071e4ee70a2b83120ffc8dccf6f482d04358a Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Thu, 15 Feb 2024 13:28:25 -0500 Subject: [PATCH 02/31] Implement RuntimeError for overtaken cvars --- core/src/main/java/org/jruby/RubyModule.java | 6 ++++++ spec/tags/ruby/language/class_variable_tags.txt | 1 - 2 files changed, 6 insertions(+), 1 deletion(-) delete mode 100644 spec/tags/ruby/language/class_variable_tags.txt diff --git a/core/src/main/java/org/jruby/RubyModule.java b/core/src/main/java/org/jruby/RubyModule.java index f58e3a8720b..7ed85c73c71 100644 --- a/core/src/main/java/org/jruby/RubyModule.java +++ b/core/src/main/java/org/jruby/RubyModule.java @@ -4602,13 +4602,19 @@ public IRubyObject getClassVarQuiet(String name) { assert IdUtil.isClassVariable(name); RubyModule module = this; RubyModule highest = null; + RubyModule lowest = null; do { if (module.hasClassVariable(name)) { highest = module; + if (lowest == null) lowest = module; } } while ((module = module.getSuperClass()) != null); + if (lowest != highest) { + throw getRuntime().newRuntimeError(str(getRuntime(), "class variable " + name + " of ", lowest, " is overtaken by ", highest)); + } + if (highest != null) return highest.fetchClassVariable(name); return null; diff --git a/spec/tags/ruby/language/class_variable_tags.txt b/spec/tags/ruby/language/class_variable_tags.txt deleted file mode 100644 index 8960545d44f..00000000000 --- a/spec/tags/ruby/language/class_variable_tags.txt +++ /dev/null @@ -1 +0,0 @@ -fails:Accessing a class variable raises a RuntimeError when a class variable is overtaken in an ancestor class From 6f3f1f2348b3a12e2cfeb35b19cf025d22a80a41 Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Tue, 27 Feb 2024 16:40:10 -0500 Subject: [PATCH 03/31] arity for {|a,| } was -2 and should have been 1 --- core/src/main/java/org/jruby/runtime/Signature.java | 3 ++- spec/tags/ruby/core/proc/arity_tags.txt | 2 -- 2 files changed, 2 insertions(+), 3 deletions(-) delete mode 100644 spec/tags/ruby/core/proc/arity_tags.txt diff --git a/core/src/main/java/org/jruby/runtime/Signature.java b/core/src/main/java/org/jruby/runtime/Signature.java index d4725735b85..f4bf8a6a340 100644 --- a/core/src/main/java/org/jruby/runtime/Signature.java +++ b/core/src/main/java/org/jruby/runtime/Signature.java @@ -135,8 +135,9 @@ public int calculateArityValue() { int oneForKeywords = requiredKwargs > 0 ? 1 : 0; int fixedValue = pre() + post() + oneForKeywords; boolean hasOptionalKeywords = kwargs - requiredKwargs > 0; + boolean optionalFromRest = rest() != Rest.NONE && rest != Rest.ANON; - if (opt() > 0 || rest() != Rest.NONE || (hasOptionalKeywords || restKwargs()) && oneForKeywords == 0) { + if (opt() > 0 || optionalFromRest || (hasOptionalKeywords || restKwargs()) && oneForKeywords == 0) { return -1 * (fixedValue + 1); } diff --git a/spec/tags/ruby/core/proc/arity_tags.txt b/spec/tags/ruby/core/proc/arity_tags.txt deleted file mode 100644 index e742e2ff5e3..00000000000 --- a/spec/tags/ruby/core/proc/arity_tags.txt +++ /dev/null @@ -1,2 +0,0 @@ -fails:Proc#arity for instances created with lambda { || } returns positive values for definition '@a = lambda { |a, | }' -fails:Proc#arity for instances created with proc { || } returns positive values for definition '@a = proc { |a, | }' From 8261c447aa8303a8a6d6900816b6d7d265062a70 Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Wed, 28 Feb 2024 09:46:25 -0500 Subject: [PATCH 04/31] After correcting arity I need to account for non-rest still spreading --- core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java | 4 +++- core/src/main/java/org/jruby/runtime/Signature.java | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java b/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java index 91e1c4762cf..e02932a7a4c 100644 --- a/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java +++ b/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java @@ -513,7 +513,9 @@ public static IRubyObject[] convertValueIntoArgArray(ThreadContext context, IRub new IRubyObject[] { value }; case 0: case 1: - return new IRubyObject[] { value }; + return signature.isSpreadable() ? + IRBlockBody.toAry(context, value) : + new IRubyObject[] { value }; } return IRBlockBody.toAry(context, value); diff --git a/core/src/main/java/org/jruby/runtime/Signature.java b/core/src/main/java/org/jruby/runtime/Signature.java index f4bf8a6a340..20fd06cb697 100644 --- a/core/src/main/java/org/jruby/runtime/Signature.java +++ b/core/src/main/java/org/jruby/runtime/Signature.java @@ -154,7 +154,7 @@ public int arityValue() { * @return true if the signature expects multiple args */ public boolean isSpreadable() { - return arityValue < -1 || arityValue > 1 || (opt > 0 && !restKwargs()); + return arityValue < -1 || arityValue > 1 || (opt > 0 && !restKwargs()) || rest == Rest.ANON; } From a41191ecaa38e8ac433a420d4161816eb1d81940 Mon Sep 17 00:00:00 2001 From: kares Date: Tue, 5 Mar 2024 14:53:35 +0100 Subject: [PATCH 05/31] [ji] support re-reifying class hierarchy --- core/src/main/java/org/jruby/RubyClass.java | 22 +++++++++++-------- .../jruby/util/ClassDefiningClassLoader.java | 2 ++ .../util/ClassDefiningJRubyClassLoader.java | 7 +++++- .../org/jruby/util/NClassClassLoader.java | 5 +++++ .../org/jruby/util/OneShotClassLoader.java | 10 +++++++++ 5 files changed, 36 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/org/jruby/RubyClass.java b/core/src/main/java/org/jruby/RubyClass.java index 116fb8b1af1..8df39a5cee6 100644 --- a/core/src/main/java/org/jruby/RubyClass.java +++ b/core/src/main/java/org/jruby/RubyClass.java @@ -1404,8 +1404,7 @@ public synchronized void reify(String classDumpDir, boolean useChildLoader) { final boolean concreteExt = java_box[0]; // calculate an appropriate name, for anonymous using inspect like format e.g. "Class:0x628fad4a" - final String name = getBaseName() != null ? getName() : - ( "Class_0x" + Integer.toHexString(System.identityHashCode(this)) ); + final String name = getBaseName() != null ? getName() : ( "Class_0x" + Integer.toHexString(System.identityHashCode(this)) ); final String javaName = "rubyobj." + StringSupport.replaceAll(name, "::", "."); final String javaPath = "rubyobj/" + StringSupport.replaceAll(name, "::", "/"); @@ -1415,8 +1414,8 @@ public synchronized void reify(String classDumpDir, boolean useChildLoader) { throw getClassRuntime().newTypeError(getName() + "'s parent class is not yet reified"); } - Class reifiedParent = RubyObject.class; - if (superClass.reifiedClass != null) reifiedParent = superClass.reifiedClass; + Class reifiedParent = superClass.reifiedClass; + if (reifiedParent == null) reifiedParent = RubyObject.class; Reificator reifier; if (concreteExt) { @@ -1427,24 +1426,29 @@ public synchronized void reify(String classDumpDir, boolean useChildLoader) { final byte[] classBytes = reifier.reify(); - final ClassDefiningClassLoader parentCL; + ClassDefiningClassLoader classLoader; // usually parent's class-loader if (parentReified.getClassLoader() instanceof OneShotClassLoader) { - parentCL = (OneShotClassLoader) parentReified.getClassLoader(); + classLoader = (OneShotClassLoader) parentReified.getClassLoader(); + + if (classLoader.hasDefinedClass(javaName)) { // class removed after being reified but parent got kept around + // while this seems like a leak the parent Java class might be GCable just hasn't been collected, yet + classLoader = new OneShotClassLoader((OneShotClassLoader) classLoader); + } } else { if (useChildLoader) { MultiClassLoader parentLoader = new MultiClassLoader(runtime.getJRubyClassLoader()); for(Loader cLoader : runtime.getInstanceConfig().getExtraLoaders()) { parentLoader.addClassLoader(cLoader.getClassLoader()); } - parentCL = new OneShotClassLoader(parentLoader); + classLoader = new OneShotClassLoader(parentLoader); } else { - parentCL = runtime.getJRubyClassLoader(); + classLoader = runtime.getJRubyClassLoader(); } } boolean nearEnd = false; // Attempt to load the name we plan to use; skip reification if it exists already (see #1229). try { - Class result = parentCL.defineClass(javaName, classBytes); + Class result = classLoader.defineClass(javaName, classBytes); dumpReifiedClass(classDumpDir, javaPath, classBytes); //Trigger initilization diff --git a/core/src/main/java/org/jruby/util/ClassDefiningClassLoader.java b/core/src/main/java/org/jruby/util/ClassDefiningClassLoader.java index f8e65293208..7d775ab54eb 100644 --- a/core/src/main/java/org/jruby/util/ClassDefiningClassLoader.java +++ b/core/src/main/java/org/jruby/util/ClassDefiningClassLoader.java @@ -6,6 +6,8 @@ public interface ClassDefiningClassLoader { Class loadClass(String name) throws ClassNotFoundException; + boolean hasDefinedClass(String name); + default ClassLoader asClassLoader() { return (ClassLoader) this; } } diff --git a/core/src/main/java/org/jruby/util/ClassDefiningJRubyClassLoader.java b/core/src/main/java/org/jruby/util/ClassDefiningJRubyClassLoader.java index 164bcc97ca0..0334c8e3ee5 100644 --- a/core/src/main/java/org/jruby/util/ClassDefiningJRubyClassLoader.java +++ b/core/src/main/java/org/jruby/util/ClassDefiningJRubyClassLoader.java @@ -73,6 +73,11 @@ public Class defineClass(String name, byte[] bytes, ProtectionDomain domain) * @return whether it's loadable */ public boolean hasClass(String name) { - return definedClasses.contains(name) || super.findResource(name.replace('.', '/') + ".class") != null; + return hasDefinedClass(name) || super.findResource(name.replace('.', '/') + ".class") != null; + } + + @Override + public boolean hasDefinedClass(String name) { + return definedClasses.contains(name); } } diff --git a/core/src/main/java/org/jruby/util/NClassClassLoader.java b/core/src/main/java/org/jruby/util/NClassClassLoader.java index a6e63a5bcd0..cae5f7f2d8d 100644 --- a/core/src/main/java/org/jruby/util/NClassClassLoader.java +++ b/core/src/main/java/org/jruby/util/NClassClassLoader.java @@ -14,6 +14,11 @@ public Class defineClass(String name, byte[] bytes) { return super.defineClass(name, bytes, 0, bytes.length, ClassDefiningJRubyClassLoader.DEFAULT_DOMAIN); } + @Override + public boolean hasDefinedClass(String name) { + return super.findLoadedClass(name) != null; + } + public boolean isFull() { return i >= size; } diff --git a/core/src/main/java/org/jruby/util/OneShotClassLoader.java b/core/src/main/java/org/jruby/util/OneShotClassLoader.java index c04804b76c2..b42510ed25e 100644 --- a/core/src/main/java/org/jruby/util/OneShotClassLoader.java +++ b/core/src/main/java/org/jruby/util/OneShotClassLoader.java @@ -21,4 +21,14 @@ public Class defineClass(String name, byte[] bytes) { resolveClass(cls); return cls; } + + @Override + public Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + return super.loadClass(name, resolve); + } + + @Override + public boolean hasDefinedClass(String name) { + return super.findLoadedClass(name) != null; + } } From ed320c73484e44ef260bd7da4402907ecd286ad1 Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Wed, 6 Mar 2024 15:56:29 -0500 Subject: [PATCH 06/31] Fixed some stuff but not enough --- core/src/main/java/org/jruby/RubyModule.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/jruby/RubyModule.java b/core/src/main/java/org/jruby/RubyModule.java index b6c25c111b5..ab45036d286 100644 --- a/core/src/main/java/org/jruby/RubyModule.java +++ b/core/src/main/java/org/jruby/RubyModule.java @@ -4619,7 +4619,13 @@ public IRubyObject getClassVarQuiet(String name) { } while ((module = module.getSuperClass()) != null); if (lowest != highest) { - throw getRuntime().newRuntimeError(str(getRuntime(), "class variable " + name + " of ", lowest, " is overtaken by ", highest)); + if (lowest.getOrigin().getRealModule() != highest.getOrigin().getRealModule()) { + throw getRuntime().newRuntimeError(str(getRuntime(), "class variable " + name + " of ", lowest, " is overtaken by ", highest)); + } + /* + if (!(lowest instanceof IncludedModule || lowest instanceof PrependedModule)) { + lowest.removeClassVariable(name); + }*/ } if (highest != null) return highest.fetchClassVariable(name); From 1b7fdff84be63ce0ed9fb95f67e3abf6491d731f Mon Sep 17 00:00:00 2001 From: kares Date: Thu, 7 Mar 2024 08:08:49 +0100 Subject: [PATCH 07/31] [test] just reformatting since it's hard to read --- .../reify/become_java_spec.rb | 303 +++++++++--------- 1 file changed, 150 insertions(+), 153 deletions(-) diff --git a/spec/java_integration/reify/become_java_spec.rb b/spec/java_integration/reify/become_java_spec.rb index b42fe43f397..81bcf7559df 100644 --- a/spec/java_integration/reify/become_java_spec.rb +++ b/spec/java_integration/reify/become_java_spec.rb @@ -118,174 +118,171 @@ class TopRightOfTheJStack < MiddleOfTheJStack ;def size; super + 3; end ; end end it "constructs in the right order using the right methods" do - class BottomOfTheCJStack < java.util.ArrayList - def initialize(lst) - lst << :bottom_start - super() - lst << :bottom_end - end - end - class MiddleLofTheCJStack < BottomOfTheCJStack - def initialize(lst) - lst << :mid1_start - super - lst << :mid1_end - end - end - class MiddleMofTheCJStack < MiddleLofTheCJStack - def initialize(lst) - lst << :mid2_error - super - lst << :mid2_end_error - end - configure_java_class ctor_name: :reconfigured - def reconfigured(lst) - lst << :mid2_good_start - super - lst << :mid2_good_end - end - end - class MiddleUofTheCJStack < MiddleMofTheCJStack - #default init - end - class TopOfTheCJStack < MiddleUofTheCJStack - def initialize(lst) - lst << :top_start - super - lst << :top_end - end - end + class BottomOfTheCJStack < java.util.ArrayList + def initialize(lst) + lst << :bottom_start + super() + lst << :bottom_end + end + end + class MiddleLofTheCJStack < BottomOfTheCJStack + def initialize(lst) + lst << :mid1_start + super + lst << :mid1_end + end + end + class MiddleMofTheCJStack < MiddleLofTheCJStack + def initialize(lst) + lst << :mid2_error + super + lst << :mid2_end_error + end + configure_java_class ctor_name: :reconfigured + def reconfigured(lst) + lst << :mid2_good_start + super + lst << :mid2_good_end + end + end + class MiddleUofTheCJStack < MiddleMofTheCJStack + #default init + end + class TopOfTheCJStack < MiddleUofTheCJStack + def initialize(lst) + lst << :top_start + super + lst << :top_end + end + end - trace = [] + trace = [] expect(TopOfTheCJStack.new(trace)).not_to be_nil expect(trace).to eql([:top_start, :mid2_good_start, :mid1_start, :bottom_start, :bottom_end, :mid1_end, :mid2_good_end, :top_end]) end it "supports reification of java classes with interfaces" do - clz = Class.new(java.lang.Exception) do - include java.util.Iterator - def initialize(array) - @array = array - super(array.inspect) - @at=0 - @loop = false - end - def hasNext - @at<@array.length - end - def next() - @array[@array.length - 1 - @at].tap{@at+=1} - end - - def remove - raise java.lang.StackOverflowError.new if @loop - @loop = true - begin - @array<< :fail1 - super - @array << :fail2 - rescue java.lang.UnsupportedOperationException => uo - @array = [:success] - rescue java.lang.StackOverflowError => so - @array << :failSO - end - @loop = false - end - end - - gotten = [] - clz.new([:a, :b, :c]).forEachRemaining { |k| gotten << k } - expect(gotten).to eql([:c,:b, :a]) - expect(clz.new([:a, :b, :c]).message).to eql("[:a, :b, :c]") - - pending "GH#6479 + reification not yet hooked up" - obj = clz.new(["fail3"]) - obj.remove - gotten = [] - obj.forEachRemaining { |k| gotten << k } - expect(gotten).to eql([:success]) - expect(gotten.length).to eql(2) + clz = Class.new(java.lang.Exception) do + include java.util.Iterator + def initialize(array) + @array = array + super(array.inspect) + @at = 0 + @loop = false + end + def hasNext + @at < @array.length + end + def next() + @array[@array.length - 1 - @at].tap{@at+=1} + end + def remove + raise java.lang.StackOverflowError.new if @loop + @loop = true + begin + @array<< :fail1 + super + @array << :fail2 + rescue java.lang.UnsupportedOperationException => uo + @array = [:success] + rescue java.lang.StackOverflowError => so + @array << :failSO + end + @loop = false + end + end + + gotten = [] + clz.new([:a, :b, :c]).forEachRemaining { |k| gotten << k } + expect(gotten).to eql([:c,:b, :a]) + expect(clz.new([:a, :b, :c]).message).to eql("[:a, :b, :c]") + + pending "GH#6479 + reification not yet hooked up" + obj = clz.new(["fail3"]) + obj.remove + gotten = [] + obj.forEachRemaining { |k| gotten << k } + expect(gotten).to eql([:success]) + expect(gotten.length).to eql(2) end it "supports reification of ruby classes with interfaces" do pending "GH#6479 + reification not yet hooked up" - clz = Class.new do - include java.util.Iterator - def initialize(array) - @array = array - @at=0 - @loop = false - end - def hasNext - @at<@array.length - end - def next() - @array[@array.length - 1 - @at].tap{@at+=1} - end - - def remove - raise java.lang.StackOverflowError.new if @loop - @loop = true - begin - @array<< :fail1 - super - @array << :fail2 - rescue java.lang.UnsupportedOperationException => uo - @array = [:success] - rescue java.lang.StackOverflowError => so - @array << :failSO - end - @loop = false - end - end - - obj = clz.new(["fail3"]) - obj.remove - gotten = [] - obj.forEachRemaining { |k| gotten << k } - expect(gotten).to eql([:success]) - expect(gotten.length).to eql(2) + clz = Class.new do + include java.util.Iterator + def initialize(array) + @array = array + @at = 0 + @loop = false + end + def hasNext + @at < @array.length + end + def next() + @array[@array.length - 1 - @at].tap{@at+=1} + end + + def remove + raise java.lang.StackOverflowError.new if @loop + @loop = true + begin + @array<< :fail1 + super + @array << :fail2 + rescue java.lang.UnsupportedOperationException => uo + @array = [:success] + rescue java.lang.StackOverflowError => so + @array << :failSO + end + @loop = false + end + end + + obj = clz.new(["fail3"]) + obj.remove + gotten = [] + obj.forEachRemaining { |k| gotten << k } + expect(gotten).to eql([:success]) + expect(gotten.length).to eql(2) end - it "supports reification of concrete classes with non-standard construction" do - - clz = Class.new(java.lang.Exception) do - def jinit(str, seq) - @seq = seq - super("foo: #{str}") - seq << :jinit - end - - def initialize(str, foo) - @seq << :init - @seq << str - @seq << foo - end - - def self.new(seq, str) - obj = allocate - seq << :new - obj.__jallocate!(str, seq) - seq << :ready - obj - end - - configure_java_class ctor_name: :jinit - end - - bclz = clz.become_java! + clz = Class.new(java.lang.Exception) do + def jinit(str, seq) + @seq = seq + super("foo: #{str}") + seq << :jinit + end + + def initialize(str, foo) + @seq << :init + @seq << str + @seq << foo + end + + def self.new(seq, str) + obj = allocate + seq << :new + obj.__jallocate!(str, seq) + seq << :ready + obj + end + + configure_java_class ctor_name: :jinit + end - lst = [] - obj = clz.new(lst, :bar) - expect(obj).not_to be_nil - expect(lst).to eq([:new, :jinit, :ready]) - expect(obj.message).to eq("foo: bar") - obj.send :initialize, :x, "y" - expect(lst).to eq([:new, :jinit, :ready, :init, :x, "y"]) - expect(bclz).to eq(clz.java_class) - expect(bclz).not_to eq(java.lang.Exception.java_class) + bclz = clz.become_java! + + lst = [] + obj = clz.new(lst, :bar) + expect(obj).not_to be_nil + expect(lst).to eq([:new, :jinit, :ready]) + expect(obj.message).to eq("foo: bar") + obj.send :initialize, :x, "y" + expect(lst).to eq([:new, :jinit, :ready, :init, :x, "y"]) + expect(bclz).to eq(clz.java_class) + expect(bclz).not_to eq(java.lang.Exception.java_class) end it "supports reification of annotations and signatures on static methods without parameters" do From e3aacb5fe37a796d59508aefb150e792ac18a4b7 Mon Sep 17 00:00:00 2001 From: kares Date: Thu, 7 Mar 2024 08:21:22 +0100 Subject: [PATCH 08/31] [test] case for the bug report at GH-8141 --- .../reify/become_java_spec.rb | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/spec/java_integration/reify/become_java_spec.rb b/spec/java_integration/reify/become_java_spec.rb index 81bcf7559df..9543c7e7c14 100644 --- a/spec/java_integration/reify/become_java_spec.rb +++ b/spec/java_integration/reify/become_java_spec.rb @@ -116,6 +116,39 @@ class TopRightOfTheJStack < MiddleOfTheJStack ;def size; super + 3; end ; end expect(TopLeftOfTheJStack.new.size).to eq 0 expect(TopRightOfTheJStack.new([:a, :b]).size).to eq (2+3) end + + it "supports auto reifying a class hierarchy when class gets redefined" do + class ASubList < java.util.ArrayList + attr_reader :args + def initialize(arg1, arg2) + @args = [arg1, arg2] + super(arg1 + arg2) + end + end + class ASubSubList < ASubList; end + expect( ASubSubList.new(1, 2).args ).to eql [1, 2] + + Object.send(:remove_const, :ASubSubList) + + class ASubSubList < ASubList; end + expect( ASubSubList.new(1, 2).args ).to eql [1, 2] + + Object.send(:remove_const, :ASubSubList) + Object.send(:remove_const, :ASubList) + + class ASubList < java.util.ArrayList + attr_reader :args + def initialize(arg1, arg2) + @args = [arg2, arg1] + super(arg1 + arg2) + end + end + class ASubSubList < ASubList; end + expect( ASubSubList.new(1, 2).args ).to eql [2, 1] + + Object.send(:remove_const, :ASubSubList) + Object.send(:remove_const, :ASubList) + end it "constructs in the right order using the right methods" do class BottomOfTheCJStack < java.util.ArrayList From a36a465d849fac074a7e6d7d5ea473b64c8bd4c0 Mon Sep 17 00:00:00 2001 From: kares Date: Thu, 7 Mar 2024 13:25:06 +0100 Subject: [PATCH 09/31] [fix] re-invented without juggling around with class loaders --- core/src/main/java/org/jruby/RubyClass.java | 26 +++++++++++-------- core/src/main/java/org/jruby/RubyModule.java | 10 +++---- .../reify/become_java_spec.rb | 17 +++++++----- 3 files changed, 30 insertions(+), 23 deletions(-) diff --git a/core/src/main/java/org/jruby/RubyClass.java b/core/src/main/java/org/jruby/RubyClass.java index 8df39a5cee6..2dad0facc67 100644 --- a/core/src/main/java/org/jruby/RubyClass.java +++ b/core/src/main/java/org/jruby/RubyClass.java @@ -1401,13 +1401,10 @@ public synchronized void reify(String classDumpDir, boolean useChildLoader) { boolean[] java_box = { false }; // re-check reifiable in case another reify call has jumped in ahead of us if (!isReifiable(java_box)) return; - final boolean concreteExt = java_box[0]; + final boolean concreteExt = java_box[0]; - // calculate an appropriate name, for anonymous using inspect like format e.g. "Class:0x628fad4a" - final String name = getBaseName() != null ? getName() : ( "Class_0x" + Integer.toHexString(System.identityHashCode(this)) ); - - final String javaName = "rubyobj." + StringSupport.replaceAll(name, "::", "."); - final String javaPath = "rubyobj/" + StringSupport.replaceAll(name, "::", "/"); + final String javaName = getReifiedJavaClassName(); + final String javaPath = javaName.replace('.', '/'); final Class parentReified = superClass.getRealClass().getReifiedClass(); if (parentReified == null) { @@ -1429,11 +1426,6 @@ public synchronized void reify(String classDumpDir, boolean useChildLoader) { ClassDefiningClassLoader classLoader; // usually parent's class-loader if (parentReified.getClassLoader() instanceof OneShotClassLoader) { classLoader = (OneShotClassLoader) parentReified.getClassLoader(); - - if (classLoader.hasDefinedClass(javaName)) { // class removed after being reified but parent got kept around - // while this seems like a leak the parent Java class might be GCable just hasn't been collected, yet - classLoader = new OneShotClassLoader((OneShotClassLoader) classLoader); - } } else { if (useChildLoader) { MultiClassLoader parentLoader = new MultiClassLoader(runtime.getJRubyClassLoader()); @@ -1500,6 +1492,18 @@ public synchronized void reify(String classDumpDir, boolean useChildLoader) { } } + private String getReifiedJavaClassName() { + final String basePackagePrefix = "rubyobj."; + if (getBaseName() == null) { // anonymous Class instance: rubyobj.Class$0x1234abcd + return basePackagePrefix + anonymousMetaNameWithIdentifier().replace(':', '$'); + } + final String identityHash = Integer.toHexString(System.identityHashCode(this)); + final CharSequence name = StringSupport.replaceAll(getName(), "::", "."); + // we need to include a Class identifier in the Java class name, since a Ruby class might be dropped + // (using remove_const) and re-created in which case using the same name would cause a conflict... + return basePackagePrefix + identityHash + '.' + name; // TheFoo::Bar -> rubyobj.320efdf5.TheFoo.Bar + } + interface Reificator { byte[] reify(); } // interface Reificator diff --git a/core/src/main/java/org/jruby/RubyModule.java b/core/src/main/java/org/jruby/RubyModule.java index e98dbfd8a4d..40c293fb514 100644 --- a/core/src/main/java/org/jruby/RubyModule.java +++ b/core/src/main/java/org/jruby/RubyModule.java @@ -795,17 +795,17 @@ private String calculateName() { } private String calculateAnonymousName() { - String cachedName = this.cachedName; // re-use cachedName field since it won't be set for anonymous class + String cachedName = this.cachedName; if (cachedName == null) { // anonymous classes get the # format - StringBuilder anonBase = new StringBuilder(24); - anonBase.append("#<").append(metaClass.getRealClass().getName()).append(":0x"); - anonBase.append(Integer.toHexString(System.identityHashCode(this))).append('>'); - cachedName = this.cachedName = anonBase.toString(); + cachedName = this.cachedName = "#<" + anonymousMetaNameWithIdentifier() + '>'; } return cachedName; } + String anonymousMetaNameWithIdentifier() { + return metaClass.getRealClass().getName() + ":0x" + Integer.toHexString(System.identityHashCode(this)); + } @JRubyMethod(name = "refine", reads = SCOPE) public IRubyObject refine(ThreadContext context, IRubyObject klass, Block block) { diff --git a/spec/java_integration/reify/become_java_spec.rb b/spec/java_integration/reify/become_java_spec.rb index 9543c7e7c14..e9c1bc4e050 100644 --- a/spec/java_integration/reify/become_java_spec.rb +++ b/spec/java_integration/reify/become_java_spec.rb @@ -22,21 +22,24 @@ def run it "uses the nesting of the class for its package name" do class ReifyInterfacesClass1 - class ReifyInterfacesClass2 + module Nested + class InnerClass + end end end ReifyInterfacesClass1.become_java! - ReifyInterfacesClass1::ReifyInterfacesClass2.become_java! + ReifyInterfacesClass1::Nested::InnerClass.become_java! jclass = java.lang.Class - expect(ReifyInterfacesClass1.to_java(jclass).name).to eq("rubyobj.ReifyInterfacesClass1") - expect(ReifyInterfacesClass1::ReifyInterfacesClass2.to_java(jclass).name).to eq("rubyobj.ReifyInterfacesClass1.ReifyInterfacesClass2") + outer_java_class_name = ReifyInterfacesClass1.to_java(jclass).name + expect(outer_java_class_name).to match /^rubyobj\.(\w)*?\.ReifyInterfacesClass1$/ + expect(ReifyInterfacesClass1::Nested::InnerClass.to_java(jclass).name).to match /^rubyobj\.(\w)*?\.ReifyInterfacesClass1\.Nested\.InnerClass$/ # checking that the packages are valid for Java's purposes expect do ReifyInterfacesClass1.new - ReifyInterfacesClass1::ReifyInterfacesClass2.new + ReifyInterfacesClass1::Nested::InnerClass.new end.not_to raise_error end @@ -465,11 +468,11 @@ def ola(*args); "OLA #{args.join(' ')}" end it 'has a similar Java class name' do ReifiedSample.become_java! klass = ReifiedSample.java_class - expect( klass.getName ).to eql 'rubyobj.ReifiedSample' + expect( klass.getName ).to match /rubyobj\.(\w)*?\.ReifiedSample/ klass = Class.new(ReifiedSample) hexid = klass.inspect.match(/(0x[0-9a-f]+)/)[1] klass = klass.become_java! - expect( klass.getName ).to match /^rubyobj\.Class_#{hexid}/ # rubyobj.Class_0x599f1b7 + expect( klass.getName ).to match /^rubyobj\.Class\$#{hexid}/ # rubyobj.Class$0x599f1b7 end it 'works when reflecting annotations' do From c3a35249d35475663f79635e39302667d3d61172 Mon Sep 17 00:00:00 2001 From: kares Date: Thu, 7 Mar 2024 14:24:46 +0100 Subject: [PATCH 10/31] [test] relax Java class-name expectation --- test/jruby/test_higher_javasupport.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/jruby/test_higher_javasupport.rb b/test/jruby/test_higher_javasupport.rb index a3f07f924c9..806eda23a18 100644 --- a/test/jruby/test_higher_javasupport.rb +++ b/test/jruby/test_higher_javasupport.rb @@ -50,7 +50,7 @@ def self.method2; end def test_passing_a_java_class_auto_reifies assert_nil Klass2.to_java.getReifiedClass # previously TestHelper.getClassName(Klass2) returned 'org.jruby.RubyObject' - assert_equal 'rubyobj.TestHigherJavasupport.Klass2', org.jruby.test.TestHelper.getClassName(Klass2) + assert org.jruby.test.TestHelper.getClassName(Klass2).end_with?('.TestHigherJavasupport.Klass2') assert_not_nil Klass2.to_java.getReifiedClass assert_not_nil Klass1.to_java.getReifiedClass end From b33c430c07994c9096831941b030512b8e14e18f Mon Sep 17 00:00:00 2001 From: kares Date: Tue, 2 Apr 2024 14:25:58 +0200 Subject: [PATCH 11/31] [fix] re-reification again - versioning the class name but starting with a simple name that is backwards compatible --- core/src/main/java/org/jruby/RubyClass.java | 45 +++++++++++-------- .../reify/become_java_spec.rb | 6 +-- 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/core/src/main/java/org/jruby/RubyClass.java b/core/src/main/java/org/jruby/RubyClass.java index 2dad0facc67..6c687fbf779 100644 --- a/core/src/main/java/org/jruby/RubyClass.java +++ b/core/src/main/java/org/jruby/RubyClass.java @@ -1403,26 +1403,11 @@ public synchronized void reify(String classDumpDir, boolean useChildLoader) { if (!isReifiable(java_box)) return; final boolean concreteExt = java_box[0]; - final String javaName = getReifiedJavaClassName(); - final String javaPath = javaName.replace('.', '/'); - final Class parentReified = superClass.getRealClass().getReifiedClass(); if (parentReified == null) { throw getClassRuntime().newTypeError(getName() + "'s parent class is not yet reified"); } - Class reifiedParent = superClass.reifiedClass; - if (reifiedParent == null) reifiedParent = RubyObject.class; - - Reificator reifier; - if (concreteExt) { - reifier = new ConcreteJavaReifier(parentReified, javaName, javaPath); - } else { - reifier = new MethodReificator(reifiedParent, javaName, javaPath, null, javaPath); - } - - final byte[] classBytes = reifier.reify(); - ClassDefiningClassLoader classLoader; // usually parent's class-loader if (parentReified.getClassLoader() instanceof OneShotClassLoader) { classLoader = (OneShotClassLoader) parentReified.getClassLoader(); @@ -1437,6 +1422,31 @@ public synchronized void reify(String classDumpDir, boolean useChildLoader) { classLoader = runtime.getJRubyClassLoader(); } } + + String javaName = getReifiedJavaClassName(); + // *might* need to include a Class identifier in the Java class name, since a Ruby class might be dropped + // (using remove_const) and re-created in which case using the same name would cause a conflict... + if (classLoader.hasDefinedClass(javaName)) { // as Ruby class dropping is "unusual" - assume v0 to be the raw name + String versionedName; int v = 1; + // NOTE: '@' is not supported in Ruby class names thus it's safe to use as a "separator" + do { + versionedName = javaName + "@v" + (v++); // rubyobj.SomeModule.Foo@v1 + } while (classLoader.hasDefinedClass(versionedName)); + javaName = versionedName; + } + final String javaPath = javaName.replace('.', '/'); + + Reificator reifier; + if (concreteExt) { + reifier = new ConcreteJavaReifier(parentReified, javaName, javaPath); + } else { + Class reifiedParent = superClass.reifiedClass; + if (reifiedParent == null) reifiedParent = RubyObject.class; + reifier = new MethodReificator(reifiedParent, javaName, javaPath, null, javaPath); + } + + final byte[] classBytes = reifier.reify(); + boolean nearEnd = false; // Attempt to load the name we plan to use; skip reification if it exists already (see #1229). try { @@ -1497,11 +1507,8 @@ private String getReifiedJavaClassName() { if (getBaseName() == null) { // anonymous Class instance: rubyobj.Class$0x1234abcd return basePackagePrefix + anonymousMetaNameWithIdentifier().replace(':', '$'); } - final String identityHash = Integer.toHexString(System.identityHashCode(this)); final CharSequence name = StringSupport.replaceAll(getName(), "::", "."); - // we need to include a Class identifier in the Java class name, since a Ruby class might be dropped - // (using remove_const) and re-created in which case using the same name would cause a conflict... - return basePackagePrefix + identityHash + '.' + name; // TheFoo::Bar -> rubyobj.320efdf5.TheFoo.Bar + return basePackagePrefix + name; // TheFoo::Bar -> rubyobj.TheFoo.Bar } interface Reificator { diff --git a/spec/java_integration/reify/become_java_spec.rb b/spec/java_integration/reify/become_java_spec.rb index e9c1bc4e050..c3e52cc1cb9 100644 --- a/spec/java_integration/reify/become_java_spec.rb +++ b/spec/java_integration/reify/become_java_spec.rb @@ -33,8 +33,8 @@ class InnerClass jclass = java.lang.Class outer_java_class_name = ReifyInterfacesClass1.to_java(jclass).name - expect(outer_java_class_name).to match /^rubyobj\.(\w)*?\.ReifyInterfacesClass1$/ - expect(ReifyInterfacesClass1::Nested::InnerClass.to_java(jclass).name).to match /^rubyobj\.(\w)*?\.ReifyInterfacesClass1\.Nested\.InnerClass$/ + expect(outer_java_class_name).to eql('rubyobj.ReifyInterfacesClass1') + expect(ReifyInterfacesClass1::Nested::InnerClass.to_java(jclass).name).to eql('rubyobj.ReifyInterfacesClass1.Nested.InnerClass') # checking that the packages are valid for Java's purposes expect do @@ -468,7 +468,7 @@ def ola(*args); "OLA #{args.join(' ')}" end it 'has a similar Java class name' do ReifiedSample.become_java! klass = ReifiedSample.java_class - expect( klass.getName ).to match /rubyobj\.(\w)*?\.ReifiedSample/ + expect( klass.getName ).to eql('rubyobj.ReifiedSample') klass = Class.new(ReifiedSample) hexid = klass.inspect.match(/(0x[0-9a-f]+)/)[1] klass = klass.become_java! From ec94c2dd757412aee79631a9db5efff07dacb3be Mon Sep 17 00:00:00 2001 From: kares Date: Wed, 3 Apr 2024 08:34:40 +0200 Subject: [PATCH 12/31] [test] specify new Java class genearation, explicitly --- spec/java_integration/reify/become_java_spec.rb | 4 ++++ .../GH-1229_reified_parent_and_child_have_same_name_spec.rb | 6 ++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/spec/java_integration/reify/become_java_spec.rb b/spec/java_integration/reify/become_java_spec.rb index c3e52cc1cb9..7789075acbb 100644 --- a/spec/java_integration/reify/become_java_spec.rb +++ b/spec/java_integration/reify/become_java_spec.rb @@ -136,6 +136,8 @@ class ASubSubList < ASubList; end class ASubSubList < ASubList; end expect( ASubSubList.new(1, 2).args ).to eql [1, 2] + old_a_sub_list_reified_class = JRuby.reference(ASubList).reified_class + Object.send(:remove_const, :ASubSubList) Object.send(:remove_const, :ASubList) @@ -149,6 +151,8 @@ def initialize(arg1, arg2) class ASubSubList < ASubList; end expect( ASubSubList.new(1, 2).args ).to eql [2, 1] + expect(JRuby.reference(ASubList).reified_class).to_not equal(old_a_sub_list_reified_class) # new Java class generated + Object.send(:remove_const, :ASubSubList) Object.send(:remove_const, :ASubList) end diff --git a/spec/regression/GH-1229_reified_parent_and_child_have_same_name_spec.rb b/spec/regression/GH-1229_reified_parent_and_child_have_same_name_spec.rb index 1d0cf0f6672..c32f7dc276a 100644 --- a/spec/regression/GH-1229_reified_parent_and_child_have_same_name_spec.rb +++ b/spec/regression/GH-1229_reified_parent_and_child_have_same_name_spec.rb @@ -1,7 +1,7 @@ require 'jruby/core_ext' describe "A child class with the same fully-qualified name as a parent class" do - it "uses its reified parent class as its own reified class" do + it 'generates a new reified class' do module GH1229 class Foo; end end @@ -15,6 +15,8 @@ class Foo; end foo_ref = JRuby.reference(foo) foo2_ref = JRuby.reference(GH1229::Foo) - expect(foo2_ref.reified_class).to equal(foo_ref.reified_class) + expect(foo_ref.reified_class).to_not be(nil) + expect(foo2_ref.reified_class).to_not be(nil) + expect(foo2_ref.reified_class).to_not equal(foo_ref.reified_class) end end From 597ff08ac1e0d7f05f8f6cc836648725756dd7db Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Mon, 29 Apr 2024 11:18:58 -0400 Subject: [PATCH 13/31] Version 9.4.7.0 updated for release --- VERSION | 2 +- core/pom.xml | 4 ++-- lib/pom.xml | 4 ++-- pom.xml | 5 ++++- shaded/pom.xml | 2 +- 5 files changed, 10 insertions(+), 7 deletions(-) diff --git a/VERSION b/VERSION index a3e5695403b..a2f96bc5819 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -9.4.7.0-SNAPSHOT +9.4.7.0 diff --git a/core/pom.xml b/core/pom.xml index 08dd82fcf33..ece130e315c 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -12,7 +12,7 @@ DO NOT MODIFY - GENERATED CODE org.jruby jruby-parent - 9.4.7.0-SNAPSHOT + 9.4.7.0 jruby-base JRuby Base @@ -711,7 +711,7 @@ DO NOT MODIFY - GENERATED CODE org.jruby jruby-base - 9.4.7.0-SNAPSHOT + 9.4.7.0 diff --git a/lib/pom.xml b/lib/pom.xml index 05acea4a573..2743404086a 100644 --- a/lib/pom.xml +++ b/lib/pom.xml @@ -12,7 +12,7 @@ DO NOT MODIFY - GENERATED CODE org.jruby jruby-parent - 9.4.7.0-SNAPSHOT + 9.4.7.0 jruby-stdlib JRuby Lib Setup @@ -28,7 +28,7 @@ DO NOT MODIFY - GENERATED CODE org.jruby jruby-core - 9.4.7.0-SNAPSHOT + 9.4.7.0 test diff --git a/pom.xml b/pom.xml index 918f0365846..1e4e18a662d 100644 --- a/pom.xml +++ b/pom.xml @@ -16,7 +16,7 @@ DO NOT MODIFY - GENERATED CODE org.jruby jruby-parent - 9.4.7.0-SNAPSHOT + 9.4.7.0 pom JRuby JRuby is the effort to recreate the Ruby (https://www.ruby-lang.org) interpreter in Java. @@ -274,6 +274,9 @@ DO NOT MODIFY - GENERATED CODE [3.3.0,) + + No Snapshots Allowed! + diff --git a/shaded/pom.xml b/shaded/pom.xml index 2266831567e..f154286046a 100644 --- a/shaded/pom.xml +++ b/shaded/pom.xml @@ -12,7 +12,7 @@ DO NOT MODIFY - GENERATED CODE org.jruby jruby-parent - 9.4.7.0-SNAPSHOT + 9.4.7.0 jruby-core JRuby Core From 3afe3fd2f12a4dccc3022d890e7303f771091d6c Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Mon, 29 Apr 2024 15:01:50 -0400 Subject: [PATCH 14/31] Update for next development cycle --- VERSION | 2 +- core/pom.xml | 4 ++-- lib/pom.xml | 4 ++-- pom.xml | 5 +---- shaded/pom.xml | 2 +- 5 files changed, 7 insertions(+), 10 deletions(-) diff --git a/VERSION b/VERSION index a2f96bc5819..848d712a28a 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -9.4.7.0 +9.4.8.0-SNAPSHOT diff --git a/core/pom.xml b/core/pom.xml index ece130e315c..e999a8d95c6 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -12,7 +12,7 @@ DO NOT MODIFY - GENERATED CODE org.jruby jruby-parent - 9.4.7.0 + 9.4.8.0-SNAPSHOT jruby-base JRuby Base @@ -711,7 +711,7 @@ DO NOT MODIFY - GENERATED CODE org.jruby jruby-base - 9.4.7.0 + 9.4.8.0-SNAPSHOT diff --git a/lib/pom.xml b/lib/pom.xml index 2743404086a..a0a0e45e639 100644 --- a/lib/pom.xml +++ b/lib/pom.xml @@ -12,7 +12,7 @@ DO NOT MODIFY - GENERATED CODE org.jruby jruby-parent - 9.4.7.0 + 9.4.8.0-SNAPSHOT jruby-stdlib JRuby Lib Setup @@ -28,7 +28,7 @@ DO NOT MODIFY - GENERATED CODE org.jruby jruby-core - 9.4.7.0 + 9.4.8.0-SNAPSHOT test diff --git a/pom.xml b/pom.xml index 1e4e18a662d..738fc04d371 100644 --- a/pom.xml +++ b/pom.xml @@ -16,7 +16,7 @@ DO NOT MODIFY - GENERATED CODE org.jruby jruby-parent - 9.4.7.0 + 9.4.8.0-SNAPSHOT pom JRuby JRuby is the effort to recreate the Ruby (https://www.ruby-lang.org) interpreter in Java. @@ -274,9 +274,6 @@ DO NOT MODIFY - GENERATED CODE [3.3.0,) - - No Snapshots Allowed! - diff --git a/shaded/pom.xml b/shaded/pom.xml index f154286046a..0e06af25fae 100644 --- a/shaded/pom.xml +++ b/shaded/pom.xml @@ -12,7 +12,7 @@ DO NOT MODIFY - GENERATED CODE org.jruby jruby-parent - 9.4.7.0 + 9.4.8.0-SNAPSHOT jruby-core JRuby Core From 280061ecebcebefc010b3e064c3865b643ed2df6 Mon Sep 17 00:00:00 2001 From: Charles Oliver Nutter Date: Tue, 30 Apr 2024 09:22:41 -0500 Subject: [PATCH 15/31] Process chdir option sooner so execFillarg sees it This is a bit hacky due to the way we handle chdir on top of posix_spawn. The logic for making the command line use sh and cd lives in execFillarg, since it must handle the case where the virtual CWD is not the same as the JVM CWD. In order to make sure that processing also happens for incoming chdir: options, this patch pre-processes the options hash just for chdir. Fixes #8126 --- .../java/org/jruby/util/io/PopenExecutor.java | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/jruby/util/io/PopenExecutor.java b/core/src/main/java/org/jruby/util/io/PopenExecutor.java index 15143f8db43..da4a3c5c580 100644 --- a/core/src/main/java/org/jruby/util/io/PopenExecutor.java +++ b/core/src/main/java/org/jruby/util/io/PopenExecutor.java @@ -87,7 +87,7 @@ public static RubyFixnum spawn(ThreadContext context, IRubyObject[] argv) { ExecArg eargp; IRubyObject fail_str; - eargp = execargNew(context, argv, true, false); + eargp = execargNew(context, argv, context.nil, true, false); execargFixup(context, runtime, eargp); fail_str = eargp.use_shell ? eargp.command_name : eargp.command_name; @@ -110,7 +110,7 @@ public IRubyObject systemInternal(ThreadContext context, IRubyObject[] argv, Str ExecArg eargp; long pid; - eargp = execargNew(context, argv, true, true); + eargp = execargNew(context, argv, context.nil, true, true); execargFixup(context, runtime, eargp); pid = spawnProcess(context, runtime, eargp, errmsg); @@ -297,7 +297,7 @@ public static IRubyObject pipeOpen(ThreadContext context, IRubyObject prog, Stri ExecArg execArg = null; if (!isPopenFork(context.runtime, (RubyString)prog)) - execArg = execargNew(context, argv, true, false); + execArg = execargNew(context, argv, context.nil, true, false); return new PopenExecutor().pipeOpen(context, execArg, modestr, fmode, convconfig); } @@ -340,14 +340,14 @@ public static IRubyObject popen(ThreadContext context, IRubyObject[] argv, RubyC // #endif tmp = ((RubyArray)tmp).aryDup(); // RBASIC_CLEAR_CLASS(tmp); - eargp = execargNew(context, ((RubyArray)tmp).toJavaArray(), false, false); + eargp = execargNew(context, ((RubyArray)tmp).toJavaArray(), opt, false, false); ((RubyArray)tmp).clear(); } else { pname = pname.convertToString(); eargp = null; if (!isPopenFork(runtime, (RubyString)pname)) { IRubyObject[] pname_p = {pname}; - eargp = execargNew(context, pname_p, true, false); + eargp = execargNew(context, pname_p, opt, true, false); pname = pname_p[0]; } } @@ -1678,14 +1678,14 @@ static RubyArray checkExecRedirect1(Ruby runtime, RubyArray ary, IRubyObject key private static final int ST_STOP = 1; // rb_execarg_new - public static ExecArg execargNew(ThreadContext context, IRubyObject[] argv, boolean accept_shell, boolean allow_exc_opt) { + public static ExecArg execargNew(ThreadContext context, IRubyObject[] argv, IRubyObject optForChdir, boolean accept_shell, boolean allow_exc_opt) { ExecArg eargp = new ExecArg(); - execargInit(context, argv, accept_shell, eargp, allow_exc_opt); + execargInit(context, argv, optForChdir, accept_shell, eargp, allow_exc_opt); return eargp; } // rb_execarg_init - private static RubyString execargInit(ThreadContext context, IRubyObject[] argv, boolean accept_shell, ExecArg eargp, boolean allow_exc_opt) { + private static RubyString execargInit(ThreadContext context, IRubyObject[] argv, IRubyObject optForChdir, boolean accept_shell, ExecArg eargp, boolean allow_exc_opt) { RubyString prog, ret; IRubyObject[] env_opt = {context.nil, context.nil}; IRubyObject[][] argv_p = {argv}; @@ -1698,6 +1698,14 @@ private static RubyString execargInit(ThreadContext context, IRubyObject[] argv, optHash = optHash.dupFast(context); exception = optHash.delete(context, exceptionSym); } + + RubySymbol chdirSym = context.runtime.newSymbol("chdir"); + IRubyObject chdir; + if (!optForChdir.isNil() && (chdir = ((RubyHash) optForChdir).delete(chdirSym)) != null) { + eargp.chdirGiven = true; + eargp.chdir_dir = chdir.convertToString().toString(); + } + execFillarg(context, prog, argv_p[0], env_opt[0], env_opt[1], eargp); if (exception.isTrue()) { eargp.exception = true; From 1e4bed1df1131072877ff8d990696a620da78229 Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Tue, 30 Apr 2024 11:30:04 -0400 Subject: [PATCH 16/31] I think this will fix specs but this is probably underspecified so likely not totally right --- core/src/main/java/org/jruby/RubyModule.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/jruby/RubyModule.java b/core/src/main/java/org/jruby/RubyModule.java index ab45036d286..594751b900f 100644 --- a/core/src/main/java/org/jruby/RubyModule.java +++ b/core/src/main/java/org/jruby/RubyModule.java @@ -4619,8 +4619,10 @@ public IRubyObject getClassVarQuiet(String name) { } while ((module = module.getSuperClass()) != null); if (lowest != highest) { - if (lowest.getOrigin().getRealModule() != highest.getOrigin().getRealModule()) { - throw getRuntime().newRuntimeError(str(getRuntime(), "class variable " + name + " of ", lowest, " is overtaken by ", highest)); + if (!highest.isPrepended()) { + if (lowest.getOrigin().getRealModule() != highest.getOrigin().getRealModule()) { + throw getRuntime().newRuntimeError(str(getRuntime(), "class variable " + name + " of ", types(getRuntime(), lowest.getOrigin().getRealModule()), " is overtaken by ", types(getRuntime(), highest.getRealModule()))); + } } /* if (!(lowest instanceof IncludedModule || lowest instanceof PrependedModule)) { From 85ab4ab4316cdcebebd7a3b1a56e267247bbb580 Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Tue, 30 Apr 2024 13:05:17 -0400 Subject: [PATCH 17/31] Introduce removal of cvars from classes when mismatched. Correct error to print out correct names when erroring --- core/src/main/java/org/jruby/RubyModule.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/jruby/RubyModule.java b/core/src/main/java/org/jruby/RubyModule.java index 594751b900f..8b9e026087e 100644 --- a/core/src/main/java/org/jruby/RubyModule.java +++ b/core/src/main/java/org/jruby/RubyModule.java @@ -4621,13 +4621,13 @@ public IRubyObject getClassVarQuiet(String name) { if (lowest != highest) { if (!highest.isPrepended()) { if (lowest.getOrigin().getRealModule() != highest.getOrigin().getRealModule()) { - throw getRuntime().newRuntimeError(str(getRuntime(), "class variable " + name + " of ", types(getRuntime(), lowest.getOrigin().getRealModule()), " is overtaken by ", types(getRuntime(), highest.getRealModule()))); + throw getRuntime().newRuntimeError(str(getRuntime(), "class variable " + name + " of ", + lowest.getOrigin(), " is overtaken by ", highest.getOrigin())); } + + if (lowest.isClass()) lowest.removeClassVariable(name); } - /* - if (!(lowest instanceof IncludedModule || lowest instanceof PrependedModule)) { - lowest.removeClassVariable(name); - }*/ + } if (highest != null) return highest.fetchClassVariable(name); From d815fe182358378fa41b26f9195c0d98b3d1e90a Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Tue, 30 Apr 2024 14:24:12 -0400 Subject: [PATCH 18/31] hmm. arity 1 with anon rest gets us to have more extra logic --- core/src/main/java/org/jruby/RubyHash.java | 2 +- core/src/main/java/org/jruby/runtime/IRBlockBody.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/jruby/RubyHash.java b/core/src/main/java/org/jruby/RubyHash.java index 51a29f73ea1..372741193a0 100644 --- a/core/src/main/java/org/jruby/RubyHash.java +++ b/core/src/main/java/org/jruby/RubyHash.java @@ -1624,7 +1624,7 @@ public RubyHash each_pairCommon(final ThreadContext context, final Block block) public void visit(ThreadContext context, RubyHash self, IRubyObject key, IRubyObject value, int index, Block block) { if (block.type == Block.Type.LAMBDA) { block.call(context, context.runtime.newArray(key, value)); - } else if (block.getSignature().arityValue() > 1) { + } else if (block.getSignature().arityValue() > 1 || block.getSignature().isSpreadable()) { block.yieldSpecific(context, key, value); } else { block.yield(context, context.runtime.newArray(key, value)); diff --git a/core/src/main/java/org/jruby/runtime/IRBlockBody.java b/core/src/main/java/org/jruby/runtime/IRBlockBody.java index 6ba10157600..eef96ab07fe 100644 --- a/core/src/main/java/org/jruby/runtime/IRBlockBody.java +++ b/core/src/main/java/org/jruby/runtime/IRBlockBody.java @@ -103,7 +103,7 @@ public IRubyObject yieldSpecific(ThreadContext context, Block block, IRubyObject private IRubyObject yieldSpecificMultiArgsCommon(ThreadContext context, Block block, IRubyObject... args) { int blockArity = signature.arityValue(); - if (blockArity == 1) { + if (blockArity == 1 && !signature.isSpreadable()) { args = new IRubyObject[] { RubyArray.newArrayMayCopy(context.runtime, args) }; } From 92b1d86718af9196f369307df97462a36ed7666a Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Tue, 30 Apr 2024 15:15:01 -0400 Subject: [PATCH 19/31] ACP needs to know about spreadable + arity of 1 --- .../java/org/jruby/ir/passes/AddCallProtocolInstructions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/jruby/ir/passes/AddCallProtocolInstructions.java b/core/src/main/java/org/jruby/ir/passes/AddCallProtocolInstructions.java index a8a5ab99624..9665ddf08b3 100644 --- a/core/src/main/java/org/jruby/ir/passes/AddCallProtocolInstructions.java +++ b/core/src/main/java/org/jruby/ir/passes/AddCallProtocolInstructions.java @@ -121,7 +121,7 @@ public Object execute(FullInterpreterContext fic, Object... data) { Signature sig = ((IRClosure)fic.getScope()).getSignature(); if (sig.isNoArguments()) { prologueBB.addInstr(PrepareNoBlockArgsInstr.INSTANCE); - } else if (sig.isOneArgument() && !sig.hasKwargs()) { // no kwargs and just a single required argument + } else if (sig.isOneArgument() && !sig.isSpreadable() && !sig.hasKwargs()) { // no kwargs and just a single required argument prologueBB.addInstr(PrepareSingleBlockArgInstr.INSTANCE); } else { prologueBB.addInstr(PrepareBlockArgsInstr.INSTANCE); From 16be7285ab4ab07a287774990d230ba557fcb2e2 Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Tue, 30 Apr 2024 16:18:59 -0400 Subject: [PATCH 20/31] Some reduction is redundant checks --- core/src/main/java/org/jruby/RubyHash.java | 2 +- .../org/jruby/ir/passes/AddCallProtocolInstructions.java | 2 +- .../src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java | 2 +- core/src/main/java/org/jruby/runtime/IRBlockBody.java | 5 ++--- core/src/main/java/org/jruby/runtime/Signature.java | 2 +- 5 files changed, 6 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/jruby/RubyHash.java b/core/src/main/java/org/jruby/RubyHash.java index 372741193a0..8c6e307f149 100644 --- a/core/src/main/java/org/jruby/RubyHash.java +++ b/core/src/main/java/org/jruby/RubyHash.java @@ -1624,7 +1624,7 @@ public RubyHash each_pairCommon(final ThreadContext context, final Block block) public void visit(ThreadContext context, RubyHash self, IRubyObject key, IRubyObject value, int index, Block block) { if (block.type == Block.Type.LAMBDA) { block.call(context, context.runtime.newArray(key, value)); - } else if (block.getSignature().arityValue() > 1 || block.getSignature().isSpreadable()) { + } else if (block.getSignature().isSpreadable()) { block.yieldSpecific(context, key, value); } else { block.yield(context, context.runtime.newArray(key, value)); diff --git a/core/src/main/java/org/jruby/ir/passes/AddCallProtocolInstructions.java b/core/src/main/java/org/jruby/ir/passes/AddCallProtocolInstructions.java index 9665ddf08b3..a8a5ab99624 100644 --- a/core/src/main/java/org/jruby/ir/passes/AddCallProtocolInstructions.java +++ b/core/src/main/java/org/jruby/ir/passes/AddCallProtocolInstructions.java @@ -121,7 +121,7 @@ public Object execute(FullInterpreterContext fic, Object... data) { Signature sig = ((IRClosure)fic.getScope()).getSignature(); if (sig.isNoArguments()) { prologueBB.addInstr(PrepareNoBlockArgsInstr.INSTANCE); - } else if (sig.isOneArgument() && !sig.isSpreadable() && !sig.hasKwargs()) { // no kwargs and just a single required argument + } else if (sig.isOneArgument() && !sig.hasKwargs()) { // no kwargs and just a single required argument prologueBB.addInstr(PrepareSingleBlockArgInstr.INSTANCE); } else { prologueBB.addInstr(PrepareBlockArgsInstr.INSTANCE); diff --git a/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java b/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java index e02932a7a4c..95dbcb8b25c 100644 --- a/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java +++ b/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java @@ -513,7 +513,7 @@ public static IRubyObject[] convertValueIntoArgArray(ThreadContext context, IRub new IRubyObject[] { value }; case 0: case 1: - return signature.isSpreadable() ? + return signature.rest() == org.jruby.runtime.Signature.Rest.ANON ? IRBlockBody.toAry(context, value) : new IRubyObject[] { value }; } diff --git a/core/src/main/java/org/jruby/runtime/IRBlockBody.java b/core/src/main/java/org/jruby/runtime/IRBlockBody.java index eef96ab07fe..ae27eedf281 100644 --- a/core/src/main/java/org/jruby/runtime/IRBlockBody.java +++ b/core/src/main/java/org/jruby/runtime/IRBlockBody.java @@ -102,14 +102,13 @@ public IRubyObject yieldSpecific(ThreadContext context, Block block, IRubyObject } private IRubyObject yieldSpecificMultiArgsCommon(ThreadContext context, Block block, IRubyObject... args) { - int blockArity = signature.arityValue(); - if (blockArity == 1 && !signature.isSpreadable()) { + if (signature.isOneArgument()) { args = new IRubyObject[] { RubyArray.newArrayMayCopy(context.runtime, args) }; } if (canCallDirect()) return yieldDirect(context, block, args, null); - if (blockArity == 0) { + if (signature.arityValue() == 0) { args = IRubyObject.NULL_ARRAY; // discard args } if (block.type == Block.Type.LAMBDA) signature.checkArity(context.runtime, args); diff --git a/core/src/main/java/org/jruby/runtime/Signature.java b/core/src/main/java/org/jruby/runtime/Signature.java index 20fd06cb697..1590067cd66 100644 --- a/core/src/main/java/org/jruby/runtime/Signature.java +++ b/core/src/main/java/org/jruby/runtime/Signature.java @@ -85,7 +85,7 @@ public boolean restKwargs() { * Are there an exact (fixed) number of parameters to this signature? */ public boolean isFixed() { - return arityValue() >= 0; + return arityValue() >= 0 && rest != Rest.ANON; } /** From b0084b7963ce35978c178ac1fd02b33a3a23af12 Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Wed, 1 May 2024 10:08:06 -0400 Subject: [PATCH 21/31] SignalException is using wrong arity check (from test_arity in MRI) --- .../java/org/jruby/RubySignalException.java | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/org/jruby/RubySignalException.java b/core/src/main/java/org/jruby/RubySignalException.java index 04c9f95ebc8..fa9ed8f6e7a 100644 --- a/core/src/main/java/org/jruby/RubySignalException.java +++ b/core/src/main/java/org/jruby/RubySignalException.java @@ -62,26 +62,23 @@ static RubyClass define(Ruby runtime, RubyClass exceptionClass) { return signalExceptionClass; } - @JRubyMethod(optional = 2, checkArity = false, visibility = PRIVATE) + @JRubyMethod(required = 1, optional = 2, checkArity = false, visibility = PRIVATE) public IRubyObject initialize(ThreadContext context, IRubyObject[] args, Block block) { - int argc = Arity.checkArgumentCount(context, args, 0, 2); + int argc = Arity.checkArgumentCount(context, args, 1, 2); final Ruby runtime = context.runtime; int argnum = 1; - IRubyObject sig = context.nil; - long _signo; - if (argc > 0) { - sig = TypeConverter.checkToInteger(runtime, args[0], "to_int"); + IRubyObject sig = TypeConverter.checkToInteger(runtime, args[0], "to_int"); - if (sig.isNil()) { - sig = args[0]; - } else { - argnum = 2; - } + if (sig.isNil()) { + sig = args[0]; + Arity.checkArgumentCount(runtime, args, 1, argnum); + } else { + argnum = 2; } - Arity.checkArgumentCount(runtime, args, 1, argnum); + long _signo; if (argnum == 2) { _signo = sig.convertToInteger().getLongValue(); From 94105c25e8b6ed969ff9ea9ec5ad34ecd2de0a3d Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Wed, 1 May 2024 10:11:51 -0400 Subject: [PATCH 22/31] Hash.new(1) {} is giving wrong argument error string --- core/src/main/java/org/jruby/RubyHash.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/jruby/RubyHash.java b/core/src/main/java/org/jruby/RubyHash.java index c5b39bce15b..205ceca7001 100644 --- a/core/src/main/java/org/jruby/RubyHash.java +++ b/core/src/main/java/org/jruby/RubyHash.java @@ -814,11 +814,10 @@ public IRubyObject initialize(ThreadContext context, final Block block) { public IRubyObject initialize(ThreadContext context, IRubyObject _default, final Block block) { modify(); - if (block.isGiven()) { - throw context.runtime.newArgumentError("wrong number of arguments"); - } else { - ifNone = _default; - } + if (block.isGiven()) throw context.runtime.newArgumentError(1, 0); + + ifNone = _default; + return this; } From 4a4e30e63e4bf15be9cc28de8a7dd76a746d66b2 Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Wed, 1 May 2024 10:15:19 -0400 Subject: [PATCH 23/31] another mismatched error string --- core/src/main/ruby/jruby/kernel/jruby/process_util.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/ruby/jruby/kernel/jruby/process_util.rb b/core/src/main/ruby/jruby/kernel/jruby/process_util.rb index 7786153dcd9..b009973d390 100644 --- a/core/src/main/ruby/jruby/kernel/jruby/process_util.rb +++ b/core/src/main/ruby/jruby/kernel/jruby/process_util.rb @@ -4,7 +4,7 @@ def self.exec_args(args) env, prog, opts = nil if args.size < 1 - raise ArgumentError, 'wrong number of arguments' + raise ArgumentError, 'wrong number of arguments (given 0, expected 1+)' end # peel off options From bd638821ecadf5741a3a0a12c1c0774411943756 Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Wed, 1 May 2024 10:17:15 -0400 Subject: [PATCH 24/31] Remove tags for arity specs in MRI test suite as they pass now --- test/mri/excludes/TestArity.rb | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 test/mri/excludes/TestArity.rb diff --git a/test/mri/excludes/TestArity.rb b/test/mri/excludes/TestArity.rb deleted file mode 100644 index 760fd49eb42..00000000000 --- a/test/mri/excludes/TestArity.rb +++ /dev/null @@ -1,3 +0,0 @@ -exclude :test_message_change_issue_6085, "needs investigation" -exclude :test_method_err_mess, "needs investigation" -exclude :test_proc_err_mess, "needs investigation" \ No newline at end of file From 4c1473a794f2cd596bf9e29de9ac3ba58a501cdc Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Wed, 1 May 2024 14:14:19 -0400 Subject: [PATCH 25/31] shallow clone of binding happen in Binding#dup --- core/src/main/java/org/jruby/RubyBinding.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/jruby/RubyBinding.java b/core/src/main/java/org/jruby/RubyBinding.java index 607a821014b..7ef4618d7d8 100644 --- a/core/src/main/java/org/jruby/RubyBinding.java +++ b/core/src/main/java/org/jruby/RubyBinding.java @@ -108,7 +108,7 @@ public IRubyObject initialize(ThreadContext context) { @JRubyMethod public IRubyObject dup(ThreadContext context) { - return newBinding(context.runtime, binding); + return newBinding(context.runtime, binding.clone()); } @JRubyMethod(name = "initialize_copy", visibility = Visibility.PRIVATE) From d994845e08ff1c9a2aef1d12f70599b980b67e98 Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Wed, 1 May 2024 14:49:24 -0400 Subject: [PATCH 26/31] dup will dup binding state already saved so it will keep variable values but they will be their own thing from that point onward --- core/src/main/java/org/jruby/RubyBinding.java | 2 +- core/src/main/java/org/jruby/parser/StaticScope.java | 2 +- core/src/main/java/org/jruby/runtime/Binding.java | 7 +++++++ core/src/main/java/org/jruby/runtime/DynamicScope.java | 4 ++++ 4 files changed, 13 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/jruby/RubyBinding.java b/core/src/main/java/org/jruby/RubyBinding.java index 7ef4618d7d8..ce36b1944ca 100644 --- a/core/src/main/java/org/jruby/RubyBinding.java +++ b/core/src/main/java/org/jruby/RubyBinding.java @@ -108,7 +108,7 @@ public IRubyObject initialize(ThreadContext context) { @JRubyMethod public IRubyObject dup(ThreadContext context) { - return newBinding(context.runtime, binding.clone()); + return newBinding(context.runtime, binding.dup(context)); } @JRubyMethod(name = "initialize_copy", visibility = Visibility.PRIVATE) diff --git a/core/src/main/java/org/jruby/parser/StaticScope.java b/core/src/main/java/org/jruby/parser/StaticScope.java index 1b37fac8a3a..fb50733084d 100644 --- a/core/src/main/java/org/jruby/parser/StaticScope.java +++ b/core/src/main/java/org/jruby/parser/StaticScope.java @@ -76,7 +76,7 @@ * will point to the previous scope of the enclosing module/class (cref). * */ -public class StaticScope implements Serializable { +public class StaticScope implements Serializable, Cloneable { public static final int MAX_SPECIALIZED_SIZE = 50; private static final long serialVersionUID = 3423852552352498148L; diff --git a/core/src/main/java/org/jruby/runtime/Binding.java b/core/src/main/java/org/jruby/runtime/Binding.java index 2be702dd3fa..1cfeaa92f9c 100644 --- a/core/src/main/java/org/jruby/runtime/Binding.java +++ b/core/src/main/java/org/jruby/runtime/Binding.java @@ -316,4 +316,11 @@ public BacktraceElement getBacktrace() { // Can't use Frame.DUMMY because of circular static init seeing it before it's assigned new Frame(), Visibility.PUBLIC); + + public Binding dup(ThreadContext context) { + Binding newBinding = new Binding(this); + DynamicScope scope = getEvalScope(context.runtime); + newBinding.evalScope = scope.dup(); + return newBinding; + } } diff --git a/core/src/main/java/org/jruby/runtime/DynamicScope.java b/core/src/main/java/org/jruby/runtime/DynamicScope.java index 01ed6d18294..611db598afd 100644 --- a/core/src/main/java/org/jruby/runtime/DynamicScope.java +++ b/core/src/main/java/org/jruby/runtime/DynamicScope.java @@ -598,4 +598,8 @@ public DynamicScope cloneScope() { throw new RuntimeException("BUG: failed to clone scope type " + getClass().getName()); } } + + public DynamicScope dup() { + return newDynamicScope(staticScope.duplicate(), parent); + } } From 2b9b9d9a230742944f8ffc2b582091817ea4b3be Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Wed, 1 May 2024 15:30:20 -0400 Subject: [PATCH 27/31] bindings can change size..just making sure this fixes issues. I likely will rename this from dup --- core/src/main/java/org/jruby/runtime/DynamicScope.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/jruby/runtime/DynamicScope.java b/core/src/main/java/org/jruby/runtime/DynamicScope.java index 611db598afd..5b60f095213 100644 --- a/core/src/main/java/org/jruby/runtime/DynamicScope.java +++ b/core/src/main/java/org/jruby/runtime/DynamicScope.java @@ -32,6 +32,7 @@ import org.jruby.ir.JIT; import org.jruby.parser.StaticScope; import org.jruby.runtime.builtin.IRubyObject; +import org.jruby.runtime.scope.ManyVarsDynamicScope; public abstract class DynamicScope implements Cloneable { // Static scoping information for this scope @@ -600,6 +601,6 @@ public DynamicScope cloneScope() { } public DynamicScope dup() { - return newDynamicScope(staticScope.duplicate(), parent); + return new ManyVarsDynamicScope(staticScope.duplicate(), parent); } } From 0cc945b203133dc21842fb038a84dbc182850f7b Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Wed, 1 May 2024 16:10:15 -0400 Subject: [PATCH 28/31] dup'd binding copy all existing values into new eval scope --- core/src/main/java/org/jruby/runtime/DynamicScope.java | 7 ++++++- spec/tags/ruby/core/kernel/eval_tags.txt | 2 +- spec/tags/ruby/library/erb/result_tags.txt | 1 - spec/tags/ruby/library/erb/run_tags.txt | 1 - 4 files changed, 7 insertions(+), 4 deletions(-) delete mode 100644 spec/tags/ruby/library/erb/result_tags.txt delete mode 100644 spec/tags/ruby/library/erb/run_tags.txt diff --git a/core/src/main/java/org/jruby/runtime/DynamicScope.java b/core/src/main/java/org/jruby/runtime/DynamicScope.java index 5b60f095213..a1d75f80426 100644 --- a/core/src/main/java/org/jruby/runtime/DynamicScope.java +++ b/core/src/main/java/org/jruby/runtime/DynamicScope.java @@ -601,6 +601,11 @@ public DynamicScope cloneScope() { } public DynamicScope dup() { - return new ManyVarsDynamicScope(staticScope.duplicate(), parent); + DynamicScope newScope = new ManyVarsDynamicScope(staticScope.duplicate(), parent); + IRubyObject[] values = getValues(); + for (int i = 0; i < values.length; i++) { + newScope.setValueDepthZero(values[i], i); + } + return newScope; } } diff --git a/spec/tags/ruby/core/kernel/eval_tags.txt b/spec/tags/ruby/core/kernel/eval_tags.txt index da14ef98460..ad3754bf056 100644 --- a/spec/tags/ruby/core/kernel/eval_tags.txt +++ b/spec/tags/ruby/core/kernel/eval_tags.txt @@ -1,3 +1,3 @@ fails:Kernel#eval raises a LocalJumpError if there is no lambda-style closure in the chain fails:Kernel#eval with a magic encoding comment ignores the magic encoding comment if it is after a frozen_string_literal magic comment -fails:Kernel#eval makes flip-flop operator work correctly + diff --git a/spec/tags/ruby/library/erb/result_tags.txt b/spec/tags/ruby/library/erb/result_tags.txt deleted file mode 100644 index 0607580c953..00000000000 --- a/spec/tags/ruby/library/erb/result_tags.txt +++ /dev/null @@ -1 +0,0 @@ -fails:ERB#result use TOPLEVEL_BINDING if binding is not passed diff --git a/spec/tags/ruby/library/erb/run_tags.txt b/spec/tags/ruby/library/erb/run_tags.txt deleted file mode 100644 index 47996d0b847..00000000000 --- a/spec/tags/ruby/library/erb/run_tags.txt +++ /dev/null @@ -1 +0,0 @@ -fails:ERB#run use TOPLEVEL_BINDING if binding is not passed From 0622e7ccd4f948b76af5def27d6d5f7ca50f8dfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Duarte?= Date: Thu, 2 May 2024 09:34:39 +0100 Subject: [PATCH 29/31] Update uri to 0.12.2 for CVE-2023-36617 --- lib/pom.rb | 2 +- lib/pom.xml | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/pom.rb b/lib/pom.rb index 2be8ad44e39..909059b66f5 100644 --- a/lib/pom.rb +++ b/lib/pom.rb @@ -112,7 +112,7 @@ def log(message=nil) # ['tmpdir', '0.1.2'], ['tsort', '0.1.0'], ['un', '0.2.0'], - ['uri', '0.12.1'], + ['uri', '0.12.2'], ['weakref', '0.1.1'], # https://github.com/ruby/win32ole/issues/12 # ['win32ole', '1.8.8'], diff --git a/lib/pom.xml b/lib/pom.xml index a0a0e45e639..038b10111ff 100644 --- a/lib/pom.xml +++ b/lib/pom.xml @@ -866,7 +866,7 @@ DO NOT MODIFY - GENERATED CODE rubygems uri - 0.12.1 + 0.12.2 gem provided @@ -1143,7 +1143,7 @@ DO NOT MODIFY - GENERATED CODE specifications/timeout-0.3.2* specifications/tsort-0.1.0* specifications/un-0.2.0* - specifications/uri-0.12.1* + specifications/uri-0.12.2* specifications/weakref-0.1.1* specifications/yaml-0.2.0* specifications/matrix-0.4.2* @@ -1222,7 +1222,7 @@ DO NOT MODIFY - GENERATED CODE gems/timeout-0.3.2*/**/* gems/tsort-0.1.0*/**/* gems/un-0.2.0*/**/* - gems/uri-0.12.1*/**/* + gems/uri-0.12.2*/**/* gems/weakref-0.1.1*/**/* gems/yaml-0.2.0*/**/* gems/matrix-0.4.2*/**/* @@ -1301,7 +1301,7 @@ DO NOT MODIFY - GENERATED CODE cache/timeout-0.3.2* cache/tsort-0.1.0* cache/un-0.2.0* - cache/uri-0.12.1* + cache/uri-0.12.2* cache/weakref-0.1.1* cache/yaml-0.2.0* cache/matrix-0.4.2* From eb73e3b3708fdde9d80159c030f2f4df03dba9bf Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Thu, 2 May 2024 09:49:57 -0400 Subject: [PATCH 30/31] some docs and speed up value copying --- core/src/main/java/org/jruby/runtime/Binding.java | 9 +++++++-- .../main/java/org/jruby/runtime/DynamicScope.java | 13 +++++++------ .../jruby/runtime/scope/ManyVarsDynamicScope.java | 7 +++++++ 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/jruby/runtime/Binding.java b/core/src/main/java/org/jruby/runtime/Binding.java index 1cfeaa92f9c..7a4b24ace7a 100644 --- a/core/src/main/java/org/jruby/runtime/Binding.java +++ b/core/src/main/java/org/jruby/runtime/Binding.java @@ -317,10 +317,15 @@ public BacktraceElement getBacktrace() { new Frame(), Visibility.PUBLIC); + /** + * Duplicate this binding and setup the proper cloned instance of the eval scope so that any previously + * captured variables still exist but are not shared with the original binding. + * @param context the current thread context + * @return the duplicated binding + */ public Binding dup(ThreadContext context) { Binding newBinding = new Binding(this); - DynamicScope scope = getEvalScope(context.runtime); - newBinding.evalScope = scope.dup(); + newBinding.evalScope = getEvalScope(context.runtime).dupEvalScope(); return newBinding; } } diff --git a/core/src/main/java/org/jruby/runtime/DynamicScope.java b/core/src/main/java/org/jruby/runtime/DynamicScope.java index a1d75f80426..30f6f43808d 100644 --- a/core/src/main/java/org/jruby/runtime/DynamicScope.java +++ b/core/src/main/java/org/jruby/runtime/DynamicScope.java @@ -600,12 +600,13 @@ public DynamicScope cloneScope() { } } - public DynamicScope dup() { - DynamicScope newScope = new ManyVarsDynamicScope(staticScope.duplicate(), parent); - IRubyObject[] values = getValues(); - for (int i = 0; i < values.length; i++) { - newScope.setValueDepthZero(values[i], i); - } + /** + * Binding needs to clone its scope with all the current values. + * @return a duplicate of this scope with all the current values + */ + public DynamicScope dupEvalScope() { + ManyVarsDynamicScope newScope = new ManyVarsDynamicScope(staticScope.duplicate(), parent); + newScope.setVariableValues(getValues()); return newScope; } } diff --git a/core/src/main/java/org/jruby/runtime/scope/ManyVarsDynamicScope.java b/core/src/main/java/org/jruby/runtime/scope/ManyVarsDynamicScope.java index ea568c26d1a..2f53433fcbe 100644 --- a/core/src/main/java/org/jruby/runtime/scope/ManyVarsDynamicScope.java +++ b/core/src/main/java/org/jruby/runtime/scope/ManyVarsDynamicScope.java @@ -50,6 +50,13 @@ private void allocate() { if(variableValues == null) variableValues = IRubyObject.array(staticScope.getNumberOfVariables()); } + // WARNING: This is used when dup'ing an eval scope. We know the size and that it will 100% always be + // a ManyVarsDynamicScope. This should not be used by anything else. If there ever ends up being another + // use then it should be documented in this warning. + public void setVariableValues(IRubyObject[] variableValues) { + this.variableValues = variableValues; + } + public IRubyObject[] getValues() { return variableValues; } From 1fdac51616db076cca8a215d17c01465a25bb920 Mon Sep 17 00:00:00 2001 From: "Thomas E. Enebo" Date: Thu, 2 May 2024 10:29:42 -0400 Subject: [PATCH 31/31] Add spec to more explicitly show binding#dup behavior --- spec/ruby/core/binding/dup_spec.rb | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/spec/ruby/core/binding/dup_spec.rb b/spec/ruby/core/binding/dup_spec.rb index 55fac6e3332..f30d63b708f 100644 --- a/spec/ruby/core/binding/dup_spec.rb +++ b/spec/ruby/core/binding/dup_spec.rb @@ -10,4 +10,21 @@ bind.frozen?.should == true bind.dup.frozen?.should == false end + + it "retains original binding variables but the list is distinct" do + bind1 = binding + eval "a = 1", bind1 + + bind2 = bind1.dup + eval("a = 2", bind2) + eval("a", bind1).should == 2 + eval("a", bind2).should == 2 + + eval("b = 2", bind2) + -> { eval("b", bind1) }.should raise_error(NameError) + eval("b", bind2).should == 2 + + bind1.local_variables.sort.should == [:a, :bind1, :bind2] + bind2.local_variables.sort.should == [:a, :b, :bind1, :bind2] + end end