From 07987e57ffc0b0bcb5c3c14c75f30fb2f06ff5cb Mon Sep 17 00:00:00 2001 From: Joe Kutner Date: Thu, 9 Jul 2015 22:05:23 -0500 Subject: [PATCH 01/40] Added uri to path when using file protocol to chdir. See jruby/warbler#323 --- core/src/main/java/org/jruby/RubyDir.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/src/main/java/org/jruby/RubyDir.java b/core/src/main/java/org/jruby/RubyDir.java index 24b84c1b278..90e4807d459 100644 --- a/core/src/main/java/org/jruby/RubyDir.java +++ b/core/src/main/java/org/jruby/RubyDir.java @@ -326,6 +326,9 @@ public static IRubyObject chdir(ThreadContext context, IRubyObject recv, IRubyOb if (adjustedPath.startsWith("uri:")){ realPath = adjustedPath; } + else if (adjustedPath.startsWith("file:")){ + realPath = "uri:" + adjustedPath; + } else { FileResource dir = getDir(runtime, adjustedPath, true); realPath = dir.canonicalPath(); From 0aad2b415876214e580462ad098d184ba225912a Mon Sep 17 00:00:00 2001 From: Joe Kutner Date: Fri, 10 Jul 2015 09:38:46 -0500 Subject: [PATCH 02/40] Added PROTOCOL_PATTERN to RubyDir to mimic RubyFile --- core/src/main/java/org/jruby/RubyDir.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/jruby/RubyDir.java b/core/src/main/java/org/jruby/RubyDir.java index 90e4807d459..939ffe5115d 100644 --- a/core/src/main/java/org/jruby/RubyDir.java +++ b/core/src/main/java/org/jruby/RubyDir.java @@ -86,6 +86,8 @@ public class RubyDir extends RubyObject { private final static Encoding UTF8 = UTF8Encoding.INSTANCE; + private static Pattern PROTOCOL_PATTERN = Pattern.compile("^(uri|jar|file|classpath):([^:]*:)?//?.*"); + public RubyDir(Ruby runtime, RubyClass type) { super(runtime, type); } @@ -323,12 +325,9 @@ public static IRubyObject chdir(ThreadContext context, IRubyObject recv, IRubyOb checkDirIsTwoSlashesOnWindows(runtime, adjustedPath); String realPath = null; String oldCwd = runtime.getCurrentDirectory(); - if (adjustedPath.startsWith("uri:")){ + if (PROTOCOL_PATTERN.matcher(adjustedPath).matches()) { realPath = adjustedPath; } - else if (adjustedPath.startsWith("file:")){ - realPath = "uri:" + adjustedPath; - } else { FileResource dir = getDir(runtime, adjustedPath, true); realPath = dir.canonicalPath(); From 1c3cdaa6e74c197e07fe2f1eded792bfbaf9a116 Mon Sep 17 00:00:00 2001 From: Charles Oliver Nutter Date: Thu, 16 Jul 2015 13:56:29 -0500 Subject: [PATCH 03/40] Remove half-baked and non-functional JIT caching. Fixes #3147 --- core/src/main/java/org/jruby/Ruby.java | 29 +-------- .../java/org/jruby/RubyInstanceConfig.java | 4 +- .../java/org/jruby/compiler/JITCompiler.java | 59 +------------------ .../main/java/org/jruby/util/ClassCache.java | 16 +---- .../main/java/org/jruby/util/cli/Options.java | 4 +- 5 files changed, 8 insertions(+), 104 deletions(-) diff --git a/core/src/main/java/org/jruby/Ruby.java b/core/src/main/java/org/jruby/Ruby.java index a15eba1316e..5a1a1f0281d 100644 --- a/core/src/main/java/org/jruby/Ruby.java +++ b/core/src/main/java/org/jruby/Ruby.java @@ -819,12 +819,7 @@ private Script tryCompile(Node node, String cachedClassName, JRubyClassLoader cl String filename = node.getPosition().getFile(); String classname = JavaNameMangler.mangledFilenameForStartupClasspath(filename); - StandardASMCompiler asmCompiler = null; - if (RubyInstanceConfig.JIT_CODE_CACHE != null && cachedClassName != null) { - asmCompiler = new StandardASMCompiler(cachedClassName.replace('.', '/'), filename); - } else { - asmCompiler = new StandardASMCompiler(classname, filename); - } + StandardASMCompiler asmCompiler = new StandardASMCompiler(classname, filename); ASTCompiler compiler = config.newCompiler(); if (dump) { compiler.compileRoot(node, asmCompiler, inspector, false, false); @@ -833,11 +828,6 @@ private Script tryCompile(Node node, String cachedClassName, JRubyClassLoader cl compiler.compileRoot(node, asmCompiler, inspector, true, false); } - if (RubyInstanceConfig.JIT_CODE_CACHE != null && cachedClassName != null) { - // save script off to disk - String pathName = cachedClassName.replace('.', '/'); - JITCompiler.saveToCodeCache(this, asmCompiler.getClassByteArray(), "ruby/jit", new File(RubyInstanceConfig.JIT_CODE_CACHE, pathName + ".class")); - } script = (Script)asmCompiler.loadClass(classLoader).newInstance(); // __file__ method expects its scope at 0, so prepare that here @@ -2616,23 +2606,6 @@ public synchronized JRubyClassLoader getJRubyClassLoader() { // FIXME: Get rid of laziness and handle restricted access elsewhere if (!Ruby.isSecurityRestricted() && jrubyClassLoader == null) { jrubyClassLoader = new JRubyClassLoader(config.getLoader()); - - // if jit code cache is used, we need to add the cache directory to the classpath - // so the previously generated class files can be reused. - if( config.JIT_CODE_CACHE != null && !config.JIT_CODE_CACHE.trim().isEmpty() ) { - File file = new File( config.JIT_CODE_CACHE ); - - if( file.exists() == false || file.isDirectory() == false ) { - getWarnings().warning("The jit.codeCache '" + config.JIT_CODE_CACHE + "' directory doesn't exit."); - } else { - try { - URL url = file.toURI().toURL(); - jrubyClassLoader.addURL( url ); - } catch (MalformedURLException e) { - getWarnings().warning("Unable to add the jit.codeCache '" + config.JIT_CODE_CACHE + "' directory to the classpath." + e.getMessage()); - } - } - } } return jrubyClassLoader; diff --git a/core/src/main/java/org/jruby/RubyInstanceConfig.java b/core/src/main/java/org/jruby/RubyInstanceConfig.java index d4257254e69..2cb5e05c64b 100644 --- a/core/src/main/java/org/jruby/RubyInstanceConfig.java +++ b/core/src/main/java/org/jruby/RubyInstanceConfig.java @@ -1814,8 +1814,6 @@ public boolean shouldPrecompileAll() { public static final boolean JIT_CACHE_ENABLED = Options.JIT_CACHE.load(); - public static final String JIT_CODE_CACHE = Options.JIT_CODECACHE.load(); - public static final boolean REFLECTED_HANDLES = Options.REFLECTED_HANDLES.load(); public static final boolean NO_UNWRAP_PROCESS_STREAMS = Options.PROCESS_NOUNWRAP.load(); @@ -2030,4 +2028,6 @@ public boolean isBenchmarking() { @Deprecated public static final boolean INVOKEDYNAMIC_CONSTANTS = invokedynamicCache && Options.INVOKEDYNAMIC_CACHE_CONSTANTS.load(); @Deprecated public static final boolean INVOKEDYNAMIC_LITERALS = invokedynamicCache&& Options.INVOKEDYNAMIC_CACHE_LITERALS.load(); @Deprecated public static final boolean INVOKEDYNAMIC_IVARS = invokedynamicCache&& Options.INVOKEDYNAMIC_CACHE_IVARS.load(); + + @Deprecated public static final String JIT_CODE_CACHE = ""; } diff --git a/core/src/main/java/org/jruby/compiler/JITCompiler.java b/core/src/main/java/org/jruby/compiler/JITCompiler.java index d6ddc0c0f0f..976acaae381 100644 --- a/core/src/main/java/org/jruby/compiler/JITCompiler.java +++ b/core/src/main/java/org/jruby/compiler/JITCompiler.java @@ -96,8 +96,7 @@ public class JITCompiler implements JITCompilerMBean { private static final Logger LOG = LoggerFactory.getLogger("JITCompiler"); - - public static final boolean USE_CACHE = true; + public static final String RUBY_JIT_PREFIX = "rubyjit"; public static final String CLASS_METHOD_DELIMITER = "$$"; @@ -321,37 +320,6 @@ public static String getHashForBytes(byte[] bytes) { throw new RuntimeException(nsae); } } - - public static void saveToCodeCache(Ruby ruby, byte[] bytecode, String packageName, File cachedClassFile) { - String codeCache = RubyInstanceConfig.JIT_CODE_CACHE; - File codeCacheDir = new File(codeCache); - if (!codeCacheDir.exists()) { - ruby.getWarnings().warn("jruby.jit.codeCache directory " + codeCacheDir + " does not exist"); - } else if (!codeCacheDir.isDirectory()) { - ruby.getWarnings().warn("jruby.jit.codeCache directory " + codeCacheDir + " is not a directory"); - } else if (!codeCacheDir.canWrite()) { - ruby.getWarnings().warn("jruby.jit.codeCache directory " + codeCacheDir + " is not writable"); - } else { - if (!new File(codeCache, packageName).isDirectory()) { - boolean createdDirs = new File(codeCache, packageName).mkdirs(); - if (!createdDirs) { - ruby.getWarnings().warn("could not create JIT cache dir: " + new File(codeCache, packageName)); - } - } - // write to code cache - FileOutputStream fos = null; - try { - if (RubyInstanceConfig.JIT_LOADING_DEBUG) LOG.info("writing jitted code to to " + cachedClassFile); - fos = new FileOutputStream(cachedClassFile); - fos.write(bytecode); - } catch (Exception e) { - e.printStackTrace(); - // ignore - } finally { - try {fos.close();} catch (Exception e) {} - } - } - } public static class JITClassGenerator implements ClassCache.ClassGenerator { public JITClassGenerator(String className, String methodName, String key, Ruby ruby, DefaultMethod method, JITCounts counts) { @@ -384,27 +352,6 @@ public JITClassGenerator(String className, String methodName, String key, Ruby r protected void compile() { if (bytecode != null) return; - // check if we have a cached compiled version on disk - String codeCache = RubyInstanceConfig.JIT_CODE_CACHE; - File cachedClassFile = new File(codeCache + "/" + className + ".class"); - - if (codeCache != null && - cachedClassFile.exists()) { - FileInputStream fis = null; - try { - if (RubyInstanceConfig.JIT_LOADING_DEBUG) LOG.info("loading cached code from: " + cachedClassFile); - fis = new FileInputStream(cachedClassFile); - bytecode = new byte[(int)fis.getChannel().size()]; - fis.read(bytecode); - name = new ClassReader(bytecode).getClassName(); - return; - } catch (Exception e) { - // ignore and proceed to compile - } finally { - try {fis.close();} catch (Exception e) {} - } - } - // Time the compilation long start = System.nanoTime(); @@ -464,10 +411,6 @@ public void call(BodyCompiler context) { "JITed method size exceeds configured max of " + ruby.getInstanceConfig().getJitMaxSize()); } - - if (codeCache != null) { - JITCompiler.saveToCodeCache(ruby, bytecode, packageName, cachedClassFile); - } counts.compiledCount.incrementAndGet(); counts.compileTime.addAndGet(System.nanoTime() - start); diff --git a/core/src/main/java/org/jruby/util/ClassCache.java b/core/src/main/java/org/jruby/util/ClassCache.java index 4b9390eb8c0..fcfad54d472 100644 --- a/core/src/main/java/org/jruby/util/ClassCache.java +++ b/core/src/main/java/org/jruby/util/ClassCache.java @@ -112,23 +112,9 @@ public Class cacheClassByKey(Object key, ClassGenerator classGenerator) } protected Class defineClass(ClassGenerator classGenerator) { - // attempt to load from classloaders - String className = classGenerator.name(); - Class contents = null; - try { - contents = getClassLoader().loadClass(className); - if (RubyInstanceConfig.JIT_LOADING_DEBUG) { - LOG.debug("found jitted code in classloader: {}", className); - } - } catch (ClassNotFoundException cnfe) { - if (RubyInstanceConfig.JIT_LOADING_DEBUG) { - LOG.debug("no jitted code in classloader for method {} at class: {}", classGenerator, className); - } - // proceed to define in-memory - } OneShotClassLoader oneShotCL = new OneShotClassLoader(getClassLoader()); classGenerator.generate(); - contents = oneShotCL.defineClass(classGenerator.name(), classGenerator.bytecode()); + Class contents = oneShotCL.defineClass(classGenerator.name(), classGenerator.bytecode()); classLoadCount.incrementAndGet(); return contents; diff --git a/core/src/main/java/org/jruby/util/cli/Options.java b/core/src/main/java/org/jruby/util/cli/Options.java index c3e945b6103..7bb3760787e 100644 --- a/core/src/main/java/org/jruby/util/cli/Options.java +++ b/core/src/main/java/org/jruby/util/cli/Options.java @@ -98,7 +98,6 @@ public class Options { public static final Option JIT_LOGEVERY = integer(JIT, "jit.logEvery", 0, "Log a message every n methods JIT compiled."); public static final Option JIT_EXCLUDE = string(JIT, "jit.exclude", new String[]{"ClsOrMod","ClsOrMod::method_name","-::method_name"}, "", "Exclude methods from JIT. Comma delimited."); public static final Option JIT_CACHE = bool(JIT, "jit.cache", !COMPILE_INVOKEDYNAMIC.load(), "Cache jitted method in-memory bodies across runtimes and loads."); - public static final Option JIT_CODECACHE = string(JIT, "jit.codeCache", new String[]{"dir"}, "Save jitted methods to as they're compiled, for future runs."); public static final Option JIT_DEBUG = bool(JIT, "jit.debug", false, "Log loading of JITed bytecode."); public static final Option JIT_BACKGROUND = bool(JIT, "jit.background", true, "Run the JIT compiler in a background thread."); @@ -254,4 +253,7 @@ private static boolean calculateInvokedynamicDefault() { // We were defaulting on for Java 8 and might again later if JEP 210 helps reduce warmup time. return false; } + + @Deprecated + public static final Option JIT_CODECACHE = Option.string("jit.codeCache", JIT, new String[]{"dir"}, "Save jitted methods to as they're compiled, for future runs."); } From 01bc92cf7de7759798669dc2f5262a7ae9f1b240 Mon Sep 17 00:00:00 2001 From: Charles Oliver Nutter Date: Thu, 16 Jul 2015 16:34:42 -0500 Subject: [PATCH 04/40] Improvements to MapJavaProxy for rehash and shift. #3142 * Make the default rehash impl no-op. If the map supports rehash, the correct Java method will be bound instead. * Make the default shift have a better error explaining that Java Map does not generally preserve insertion order. --- .../main/java/org/jruby/java/proxies/MapJavaProxy.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/jruby/java/proxies/MapJavaProxy.java b/core/src/main/java/org/jruby/java/proxies/MapJavaProxy.java index 0206775619e..7d1125f11be 100644 --- a/core/src/main/java/org/jruby/java/proxies/MapJavaProxy.java +++ b/core/src/main/java/org/jruby/java/proxies/MapJavaProxy.java @@ -194,7 +194,8 @@ public void op_asetForString(Ruby runtime, RubyString key, IRubyObject value) { @Override public RubyHash rehash() { - throw getRuntime().newNotImplementedError("rehash method is not implemented in a Java Map backed object"); + // java.util.Map does not expose rehash, and many maps don't use hashing, so we do nothing. #3142 + return this; } @Override @@ -204,12 +205,9 @@ public RubyHash rb_clear() { return this; } - /** rb_hash_shift - * - */ @Override public IRubyObject shift(ThreadContext context) { - throw getRuntime().newNotImplementedError("shift method is not implemented in a Java Map backed object"); + throw getRuntime().newNotImplementedError("Java Maps do not preserve insertion order and do not support shift"); } @Override From 8e87c099f32f661c8d2245e54f510c8003563f87 Mon Sep 17 00:00:00 2001 From: Charles Oliver Nutter Date: Thu, 16 Jul 2015 16:42:38 -0500 Subject: [PATCH 05/40] Mark default java.util.Map rehash and shift as not implemented. Allows using respond_to? to check if they do anything. #3142 --- core/src/main/java/org/jruby/java/proxies/MapJavaProxy.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/jruby/java/proxies/MapJavaProxy.java b/core/src/main/java/org/jruby/java/proxies/MapJavaProxy.java index 7d1125f11be..9dd03cc7e18 100644 --- a/core/src/main/java/org/jruby/java/proxies/MapJavaProxy.java +++ b/core/src/main/java/org/jruby/java/proxies/MapJavaProxy.java @@ -312,7 +312,7 @@ public IRubyObject to_s19(ThreadContext context) { /** rb_hash_rehash * */ - @JRubyMethod(name = "rehash") + @JRubyMethod(name = "rehash", notImplemented = true) public RubyHash rehash() { return getOrCreateRubyHashMap().rehash(); } @@ -517,7 +517,7 @@ public RubyArray rb_values() { /** rb_hash_shift * */ - @JRubyMethod(name = "shift") + @JRubyMethod(name = "shift", notImplemented = true) public IRubyObject shift(ThreadContext context) { return getOrCreateRubyHashMap().shift(context); } From a5dc5b21634da529d7dbe8a76afa4b713e488a40 Mon Sep 17 00:00:00 2001 From: Charles Oliver Nutter Date: Thu, 16 Jul 2015 16:50:25 -0500 Subject: [PATCH 06/40] Properly check for uninitialized PATH env var. Fixes #3133 --- core/src/main/java/org/jruby/util/cli/ArgumentProcessor.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/jruby/util/cli/ArgumentProcessor.java b/core/src/main/java/org/jruby/util/cli/ArgumentProcessor.java index c3c7ff596e7..81932090890 100644 --- a/core/src/main/java/org/jruby/util/cli/ArgumentProcessor.java +++ b/core/src/main/java/org/jruby/util/cli/ArgumentProcessor.java @@ -562,8 +562,9 @@ private String resolveScript(String scriptName) { result = resolve("uri:classloader:/bin", scriptName); if (result != null) return result; - String path = config.getEnvironment().get("PATH").toString(); - if (path != null) { + Object maybePath = config.getEnvironment().get("PATH"); + if (maybePath != null) { + String path = maybePath.toString(); String[] paths = path.split(System.getProperty("path.separator")); for (int i = 0; i < paths.length; i++) { result = resolve(paths[i], scriptName); From ec80a3209e54cb259c45851375d0a2c2fb7334d0 Mon Sep 17 00:00:00 2001 From: Ranjeet Singh Date: Mon, 29 Jun 2015 20:13:59 +0530 Subject: [PATCH 07/40] Add specs for Date#{<<,prev_year,next_year} around calendar reforms. * See jruby/jruby#2867. --- spec/ruby/library/date/add_month_spec.rb | 8 ++++++++ spec/ruby/library/date/next_year_spec.rb | 12 ++++++++++++ spec/ruby/library/date/prev_year_spec.rb | 12 ++++++++++++ 3 files changed, 32 insertions(+) create mode 100644 spec/ruby/library/date/next_year_spec.rb create mode 100644 spec/ruby/library/date/prev_year_spec.rb diff --git a/spec/ruby/library/date/add_month_spec.rb b/spec/ruby/library/date/add_month_spec.rb index b0e23ea2291..46f1915b709 100644 --- a/spec/ruby/library/date/add_month_spec.rb +++ b/spec/ruby/library/date/add_month_spec.rb @@ -12,6 +12,14 @@ d.should == Date.civil(2008, 4, 30) end + it "returns the day of the reform if date falls within calendar reform" do + calendar_reform_italy = Date.new(1582, 10, 4) + d1 = Date.new(1582, 9, 9) >> 1 + d2 = Date.new(1582, 9, 10) >> 1 + d1.should == calendar_reform_italy + d2.should == calendar_reform_italy + end + it "raise a TypeError when passed a Symbol" do lambda { Date.civil(2007,2,27) >> :hello }.should raise_error(TypeError) end diff --git a/spec/ruby/library/date/next_year_spec.rb b/spec/ruby/library/date/next_year_spec.rb new file mode 100644 index 00000000000..70f2f7ab77d --- /dev/null +++ b/spec/ruby/library/date/next_year_spec.rb @@ -0,0 +1,12 @@ +require File.expand_path('../../../spec_helper', __FILE__) +require 'date' + +describe "Date#next_year" do + it "returns the day of the reform if date falls within calendar reform" do + calendar_reform_italy = Date.new(1582, 10, 4) + d1 = Date.new(1581, 10, 9).next_year + d2 = Date.new(1581, 10, 10).next_year + d1.should == calendar_reform_italy + d2.should == calendar_reform_italy + end +end diff --git a/spec/ruby/library/date/prev_year_spec.rb b/spec/ruby/library/date/prev_year_spec.rb new file mode 100644 index 00000000000..4f27d1d1f95 --- /dev/null +++ b/spec/ruby/library/date/prev_year_spec.rb @@ -0,0 +1,12 @@ +require File.expand_path('../../../spec_helper', __FILE__) +require 'date' + +describe "Date#prev_year" do + it "returns the day of the reform if date falls within calendar reform" do + calendar_reform_italy = Date.new(1582, 10, 4) + d1 = Date.new(1583, 10, 9).prev_year + d2 = Date.new(1583, 10, 10).prev_year + d1.should == calendar_reform_italy + d2.should == calendar_reform_italy + end +end From 40e9140209d944f424c965ccdf8b2304b8c2aade Mon Sep 17 00:00:00 2001 From: Ranjeet Singh Date: Mon, 29 Jun 2015 20:13:59 +0530 Subject: [PATCH 08/40] [#2867] modified Date#>> to take calendar reforms under consideration - added tests for next month with calendar reform - uses solution provided in #1769 comments - also fixes issues #1769 --- lib/ruby/1.9/date.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/ruby/1.9/date.rb b/lib/ruby/1.9/date.rb index eeb3fd3e078..2d5acf0ce27 100644 --- a/lib/ruby/1.9/date.rb +++ b/lib/ruby/1.9/date.rb @@ -1458,7 +1458,14 @@ def next() next_day end # of the returned Date will be the last day of the target month. def >> (n) n = n.to_int rescue raise(TypeError, "n must be a Fixnum") - self.class.new!(@dt.plusMonths(n), @of, @sg, @sub_millis) + y, m = ((year * 12) + (mon - 1) + n).divmod(12) + m, = (m + 1).divmod(1) + d = mday + until cd = _valid_civil?(y, m, d, start) + d -= 1 + raise ArgumentError, "invalid date" unless d > 0 + end + self + (cd - jd) end # Return a new Date object that is +n+ months earlier than @@ -1473,7 +1480,7 @@ def next_month(n=1) self >> n end def prev_month(n=1) self << n end def next_year(n=1) - self.class.new!(@dt.plusYears(n.to_i), @of, @sg, @sub_millis) + self >> (n * 12) end def prev_year(n=1) From a8ee690599a2d5e58c3e43af58f62b644488dcc5 Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Tue, 21 Jul 2015 12:00:37 +0200 Subject: [PATCH 09/40] Add tags for 1.8 since it is a completely different implementation. --- spec/tags/1.8/ruby/library/date/add_month_tags.txt | 1 + spec/tags/1.8/ruby/library/date/next_year_tags.txt | 1 + spec/tags/1.8/ruby/library/date/prev_year_tags.txt | 1 + 3 files changed, 3 insertions(+) create mode 100644 spec/tags/1.8/ruby/library/date/add_month_tags.txt create mode 100644 spec/tags/1.8/ruby/library/date/next_year_tags.txt create mode 100644 spec/tags/1.8/ruby/library/date/prev_year_tags.txt diff --git a/spec/tags/1.8/ruby/library/date/add_month_tags.txt b/spec/tags/1.8/ruby/library/date/add_month_tags.txt new file mode 100644 index 00000000000..f9d979847d1 --- /dev/null +++ b/spec/tags/1.8/ruby/library/date/add_month_tags.txt @@ -0,0 +1 @@ +fails:Date#>> returns the day of the reform if date falls within calendar reform diff --git a/spec/tags/1.8/ruby/library/date/next_year_tags.txt b/spec/tags/1.8/ruby/library/date/next_year_tags.txt new file mode 100644 index 00000000000..164b62d3505 --- /dev/null +++ b/spec/tags/1.8/ruby/library/date/next_year_tags.txt @@ -0,0 +1 @@ +fails:Date#next_year returns the day of the reform if date falls within calendar reform diff --git a/spec/tags/1.8/ruby/library/date/prev_year_tags.txt b/spec/tags/1.8/ruby/library/date/prev_year_tags.txt new file mode 100644 index 00000000000..7091157d1f6 --- /dev/null +++ b/spec/tags/1.8/ruby/library/date/prev_year_tags.txt @@ -0,0 +1 @@ +fails:Date#prev_year returns the day of the reform if date falls within calendar reform From 59fe0bf55dd42c5311ae1c4514bc0843d6e86609 Mon Sep 17 00:00:00 2001 From: Charles Oliver Nutter Date: Tue, 21 Jul 2015 09:41:53 -0500 Subject: [PATCH 10/40] Use iteration count for final size in map. Fixes #3155. When the block passed to map makes modifications to the array under iteration, we may prematurely finish the map loop due to the size changing. However our logic for creating the mapped array assumed the new array's size would always be the same as the original array's size, leading to an array with null elements. This fix uses the iteration count as the final size, so we at least know how many elements in the new array were populated. Note that this behavior is officially undefined; modifying the array while performing internal iteration can cause peculiar effects across runtimes and potentially across the different versions of the same runtime. We add a regression spec here to at least make sure we don't produce an invalid array. --- core/src/main/java/org/jruby/RubyArray.java | 6 ++++-- spec/regression/GH-3155_array_map_with_mutation_spec.rb | 9 +++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 spec/regression/GH-3155_array_map_with_mutation_spec.rb diff --git a/core/src/main/java/org/jruby/RubyArray.java b/core/src/main/java/org/jruby/RubyArray.java index b1ca8eabdc5..3f6cd152a2c 100644 --- a/core/src/main/java/org/jruby/RubyArray.java +++ b/core/src/main/java/org/jruby/RubyArray.java @@ -2393,13 +2393,15 @@ public IRubyObject collect(ThreadContext context, Block block) { IRubyObject[] arr = new IRubyObject[realLength]; - for (int i = 0; i < realLength; i++) { + int i; + for (i = 0; i < realLength; i++) { // Do not coarsen the "safe" check, since it will misinterpret AIOOBE from the yield // See JRUBY-5434 arr[i] = block.yield(context, safeArrayRef(values, i + begin)); } - return new RubyArray(runtime, arr); + // use iteration count as new size in case something was deleted along the way + return new RubyArray(runtime, arr, 0, i); } @JRubyMethod(name = {"collect"}, compat = RUBY1_9) diff --git a/spec/regression/GH-3155_array_map_with_mutation_spec.rb b/spec/regression/GH-3155_array_map_with_mutation_spec.rb new file mode 100644 index 00000000000..fcda19d82b5 --- /dev/null +++ b/spec/regression/GH-3155_array_map_with_mutation_spec.rb @@ -0,0 +1,9 @@ +describe "Array#map with delete inside the loop" do + it "must not produce invalid array contents" do + array = [1, 2, 3] + array2 = array.map { |v| array.delete(v); v + 1 } + + expect(array2.compact).to eq array2 + expect(array2.inspect).to eq "[2, 4]" + end +end From ff5333105c71f27c0f7b5faf16168fd5458aa24f Mon Sep 17 00:00:00 2001 From: Charles Oliver Nutter Date: Tue, 21 Jul 2015 11:25:12 -0500 Subject: [PATCH 11/40] Make Kernel#timeout redispatch to Timeout::timeout. Fixes #3158. --- .../java/org/jruby/ext/timeout/Timeout.java | 10 +--------- ...ernel_timeout_dispatches_to_module_spec.rb | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 9 deletions(-) create mode 100644 spec/regression/GH-3158_kernel_timeout_dispatches_to_module_spec.rb diff --git a/core/src/main/java/org/jruby/ext/timeout/Timeout.java b/core/src/main/java/org/jruby/ext/timeout/Timeout.java index 51b9940bfab..608292d7b52 100644 --- a/core/src/main/java/org/jruby/ext/timeout/Timeout.java +++ b/core/src/main/java/org/jruby/ext/timeout/Timeout.java @@ -85,15 +85,7 @@ public void load(Ruby runtime, boolean wrap) throws IOException { public static class TimeoutToplevel { @JRubyMethod(required = 1, optional = 1, visibility = PRIVATE) public static IRubyObject timeout(ThreadContext context, IRubyObject self, IRubyObject[] args, Block block) { - switch (args.length) { - case 1: - return Timeout.timeout(context, self, args[0], block); - case 2: - return Timeout.timeout(context, self, args[0], args[1], block); - default: - Arity.raiseArgumentError(context.runtime, args.length, 1, 2); - return context.runtime.getNil(); - } + return Helpers.invoke(context, context.runtime.getModule("Timeout"), "timeout", args, block); } } diff --git a/spec/regression/GH-3158_kernel_timeout_dispatches_to_module_spec.rb b/spec/regression/GH-3158_kernel_timeout_dispatches_to_module_spec.rb new file mode 100644 index 00000000000..41433e5d63a --- /dev/null +++ b/spec/regression/GH-3158_kernel_timeout_dispatches_to_module_spec.rb @@ -0,0 +1,20 @@ +require 'timeout' + +describe "Kernel#timeout" do + it "dynamically dispatches to Timeout::timeout" do + begin + old_verbose = $VERBOSE + $VERBOSE = nil + old_timeout = Timeout + new_timeout = Module.new do + def self.timeout(*); "foo"; end + end + Object.const_set :Timeout, new_timeout + + expect(Kernel.send(:timeout, 0)).to eq "foo" + ensure + Object.const_set :Timeout, old_timeout + $VERBOSE = old_verbose + end + end +end \ No newline at end of file From b0dcd9114bd5a1ea7ade33fdf9e2d496980f5cab Mon Sep 17 00:00:00 2001 From: kares Date: Fri, 17 Jul 2015 12:52:03 +0200 Subject: [PATCH 12/40] move find callable selector fallback into a method --- .../jruby/java/dispatch/CallableSelector.java | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java b/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java index 43bcb07082e..22cea9618bc 100644 --- a/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java +++ b/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java @@ -224,21 +224,27 @@ else if ( cType.isPrimitive() && msType.isAssignableFrom(getBoxType(cType)) ) { // fall back on old ways if (method == null) { - method = findCallable(methods, Exact, args); + method = findMatchingCallableForArgsFallback(runtime, methods, args); + } + + return method; + } + + private static T findMatchingCallableForArgsFallback(final Ruby runtime, + final T[] methods, final IRubyObject... args) { + T method = findCallable(methods, Exact, args); + if (method == null) { + method = findCallable(methods, AssignableAndPrimitivable, args); if (method == null) { - method = findCallable(methods, AssignableAndPrimitivable, args); + method = findCallable(methods, AssignableOrDuckable, args); if (method == null) { method = findCallable(methods, AssignableOrDuckable, args); if (method == null) { - method = findCallable(methods, AssignableOrDuckable, args); - if (method == null) { - method = findCallable(methods, AssignableAndPrimitivableWithVarargs, args); - } + method = findCallable(methods, AssignableAndPrimitivableWithVarargs, args); } } } } - return method; } From 581a815a3255f0d3fda86a23cdd105d7dd76d519 Mon Sep 17 00:00:00 2001 From: kares Date: Fri, 17 Jul 2015 14:26:24 +0200 Subject: [PATCH 13/40] tune proc-to-iface callable matching to do less work ... and handle proc's with arity > than the functional-interface methods parameter count --- .../jruby/java/dispatch/CallableSelector.java | 77 ++++++++++--------- .../interfaces/implementation_spec.rb | 6 ++ 2 files changed, 46 insertions(+), 37 deletions(-) diff --git a/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java b/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java index 22cea9618bc..bbc4ab75caa 100644 --- a/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java +++ b/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java @@ -166,6 +166,9 @@ private static T findMatchingCallableForArgs(final Ru Class[] msTypes = mostSpecific.getParameterTypes(); boolean ambiguous = false; + final IRubyObject lastArg = args.length > 0 ? args[ args.length - 1 ] : null; + int mostSpecificArity = 0; + OUTER: for ( int c = 1; c < size; c++ ) { final T candidate = candidates.get(c); final Class[] cTypes = candidate.getParameterTypes(); @@ -196,10 +199,43 @@ else if ( cType.isPrimitive() && msType.isAssignableFrom(getBoxType(cType)) ) { // then (int) constructor should be preferred over (float) if ( ambiguous ) { // special handling if we're dealing with Proc#impl : - final IRubyObject lastArg = args.length > 0 ? args[ args.length - 1 ] : null; - final T procToIfaceMatch = matchProcToInterfaceCandidate(lastArg, candidates); - if ( procToIfaceMatch != null ) { - mostSpecific = procToIfaceMatch; ambiguous = false; + if ( lastArg instanceof RubyProc ) { + // cases such as (both ifaces - differ in arg count) : + // java.io.File#listFiles(java.io.FileFilter) ... accept(File) + // java.io.File#listFiles(java.io.FilenameFilter) ... accept(File, String) + final int procArity = ((RubyProc) lastArg).getBlock().arity().getValue(); + + + final Method implMethod; final int cLast = cTypes.length - 1; + + if ( cLast >= 0 && cTypes[cLast].isInterface() && ( implMethod = getFunctionalInterfaceMethod(cTypes[cLast]) ) != null ) { + // we're sure to have an interface in the end - match arg count : + // NOTE: implMethod.getParameterCount() on Java 8 would do ... + final int methodArity = implMethod.getParameterTypes().length; + if ( methodArity == procArity ) { + if ( mostSpecificArity == methodArity ) ambiguous = true; // 2 with same arity + // TODO we could try to match parameter types with arg types + else { + mostSpecific = candidate; ambiguous = false; + mostSpecificArity = procArity; + } + continue /* OUTER */; // we want to check all + } + else if ( methodArity < procArity && mostSpecificArity != procArity ) { // not ideal but still usable + if ( mostSpecificArity == methodArity ) ambiguous = true; // 2 with same arity + else if ( mostSpecificArity < methodArity ) { // candidate is "better" match + mostSpecific = candidate; ambiguous = false; + mostSpecificArity = methodArity; + } + continue /* OUTER */; + } + + } + } + + //final T procToIfaceMatch = matchProcToInterfaceCandidate(lastArg, candidates); + if ( false /* && procToIfaceMatch != null */ ) { + //mostSpecific = procToIfaceMatch; ambiguous = false; } else { int msPref = 0, cPref = 0; @@ -248,39 +284,6 @@ private static T findMatchingCallableForArgsFallback( return method; } - private static T matchProcToInterfaceCandidate( - final IRubyObject lastArg, final List candidates) { - if ( lastArg instanceof RubyProc ) { - // cases such as (both ifaces - differ in arg count) : - // java.io.File#listFiles(java.io.FileFilter) ... accept(File) - // java.io.File#listFiles(java.io.FilenameFilter) ... accept(File, String) - final int arity = ((RubyProc) lastArg).getBlock().arity().getValue(); - T match = null; - for ( int i = 0; i < candidates.size(); i++ ) { - final T method = candidates.get(i); - - final Class[] params = method.getParameterTypes(); - - if ( params.length == 0 ) return null; // can not match (no args) - final Class lastParam = params[ params.length - 1 ]; - - if ( ! lastParam.isInterface() ) return null; // can not match - - final Method implMethod = getFunctionalInterfaceMethod(lastParam); - if ( implMethod != null ) { - // we're sure to have an interface in the end - match arg count : - // NOTE: implMethod.getParameterCount() on Java 8 would do ... - if ( implMethod.getParameterTypes().length == arity ) { - if ( match != null ) return null; // 2 with same arity (can not match) - match = method; // do not break here we want to check all - } - } - } - return match; - } - return null; - } - private static T findCallable(T[] callables, CallableAcceptor acceptor, IRubyObject[] args) { T bestCallable = null; int bestScore = -1; diff --git a/spec/java_integration/interfaces/implementation_spec.rb b/spec/java_integration/interfaces/implementation_spec.rb index 36d0284f8b8..e3f54a0341f 100644 --- a/spec/java_integration/interfaces/implementation_spec.rb +++ b/spec/java_integration/interfaces/implementation_spec.rb @@ -218,6 +218,12 @@ def callIt dir.should be_kind_of(java.io.File) name.should be_kind_of(String) end + + java.io.File.new('.').listFiles do |dir, name, invalid| + # should choose FilenameFilter#accept(File, String) + dir.should be_kind_of(java.io.File) + name.should be_kind_of(String) + end end it "should maintain Ruby object equality when passed through Java and back" do From 7d24ec4818c23ea4e35e49b3a41431568e83c707 Mon Sep 17 00:00:00 2001 From: kares Date: Fri, 17 Jul 2015 18:50:58 +0200 Subject: [PATCH 14/40] re-arrange proc arity matching avoids incorrect ambiguous warning e.g. in case of : `java.io.File.new('.').listFiles { |pathname| ... }` --- .../jruby/java/dispatch/CallableSelector.java | 91 +++++++++---------- 1 file changed, 43 insertions(+), 48 deletions(-) diff --git a/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java b/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java index bbc4ab75caa..b4ad28fac29 100644 --- a/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java +++ b/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java @@ -167,7 +167,7 @@ private static T findMatchingCallableForArgs(final Ru boolean ambiguous = false; final IRubyObject lastArg = args.length > 0 ? args[ args.length - 1 ] : null; - int mostSpecificArity = 0; + int mostSpecificArity = Integer.MIN_VALUE; OUTER: for ( int c = 1; c < size; c++ ) { final T candidate = candidates.get(c); @@ -195,59 +195,54 @@ else if ( cType.isPrimitive() && msType.isAssignableFrom(getBoxType(cType)) ) { } } - // somehow we can still decide e.g. if we got a RubyFixnum - // then (int) constructor should be preferred over (float) - if ( ambiguous ) { - // special handling if we're dealing with Proc#impl : - if ( lastArg instanceof RubyProc ) { - // cases such as (both ifaces - differ in arg count) : - // java.io.File#listFiles(java.io.FileFilter) ... accept(File) - // java.io.File#listFiles(java.io.FilenameFilter) ... accept(File, String) - final int procArity = ((RubyProc) lastArg).getBlock().arity().getValue(); - - - final Method implMethod; final int cLast = cTypes.length - 1; - - if ( cLast >= 0 && cTypes[cLast].isInterface() && ( implMethod = getFunctionalInterfaceMethod(cTypes[cLast]) ) != null ) { - // we're sure to have an interface in the end - match arg count : - // NOTE: implMethod.getParameterCount() on Java 8 would do ... - final int methodArity = implMethod.getParameterTypes().length; - if ( methodArity == procArity ) { - if ( mostSpecificArity == methodArity ) ambiguous = true; // 2 with same arity - // TODO we could try to match parameter types with arg types - else { - mostSpecific = candidate; ambiguous = false; - mostSpecificArity = procArity; - } - continue /* OUTER */; // we want to check all + // special handling if we're dealing with Proc#impl : + if ( lastArg instanceof RubyProc ) { + // cases such as (both ifaces - differ in arg count) : + // java.io.File#listFiles(java.io.FileFilter) ... accept(File) + // java.io.File#listFiles(java.io.FilenameFilter) ... accept(File, String) + int procArity = ((RubyProc) lastArg).getBlock().arity().getValue(); + + final Method implMethod; final int cLast = cTypes.length - 1; + + if ( cLast >= 0 && cTypes[cLast].isInterface() && ( implMethod = getFunctionalInterfaceMethod(cTypes[cLast]) ) != null ) { + // we're sure to have an interface in the end - match arg count : + // NOTE: implMethod.getParameterCount() on Java 8 would do ... + final int methodArity = implMethod.getParameterTypes().length; + if ( methodArity == procArity ) { + if ( mostSpecificArity == methodArity ) ambiguous = true; // 2 with same arity + // TODO we could try to match parameter types with arg types + else { + mostSpecific = candidate; ambiguous = false; + mostSpecificArity = procArity; } - else if ( methodArity < procArity && mostSpecificArity != procArity ) { // not ideal but still usable - if ( mostSpecificArity == methodArity ) ambiguous = true; // 2 with same arity - else if ( mostSpecificArity < methodArity ) { // candidate is "better" match - mostSpecific = candidate; ambiguous = false; - mostSpecificArity = methodArity; - } - continue /* OUTER */; + continue /* OUTER */; // we want to check all + } + else if ( methodArity < procArity && mostSpecificArity != procArity ) { // not ideal but still usable + if ( mostSpecificArity == methodArity ) ambiguous = true; // 2 with same arity + else if ( mostSpecificArity < methodArity ) { // candidate is "better" match + mostSpecific = candidate; ambiguous = false; + mostSpecificArity = methodArity; } - + continue /* OUTER */; + } + else { // we're not a match and if there's something else matched than it's not really ambiguous + ambiguous = false; /* continue /* OUTER */; } } + } - //final T procToIfaceMatch = matchProcToInterfaceCandidate(lastArg, candidates); - if ( false /* && procToIfaceMatch != null */ ) { - //mostSpecific = procToIfaceMatch; ambiguous = false; - } - else { - int msPref = 0, cPref = 0; - for ( int i = 0; i < msTypes.length; i++ ) { - final Class msType = msTypes[i], cType = cTypes[i]; - msPref += calcTypePreference(msType, args[i]); - cPref += calcTypePreference(cType, args[i]); - } - // for backwards compatibility we do not switch to cType as - // the better fit - we seem to lack tests on this front ... - if ( msPref > cPref ) ambiguous = false; // continue OUTER; + // somehow we can still decide e.g. if we got a RubyFixnum + // then (int) constructor should be preferred over (float) + if ( ambiguous ) { + int msPref = 0, cPref = 0; + for ( int i = 0; i < msTypes.length; i++ ) { + final Class msType = msTypes[i], cType = cTypes[i]; + msPref += calcTypePreference(msType, args[i]); + cPref += calcTypePreference(cType, args[i]); } + // for backwards compatibility we do not switch to cType as + // the better fit - we seem to lack tests on this front ... + if ( msPref > cPref ) ambiguous = false; // continue OUTER; } } method = mostSpecific; From e7fcfd6276ca61ac3b2884cc530a80225ea4786d Mon Sep 17 00:00:00 2001 From: kares Date: Fri, 17 Jul 2015 19:02:49 +0200 Subject: [PATCH 15/40] handle negative arity when matching proc-to-iface --- .../jruby/java/dispatch/CallableSelector.java | 1 + .../interfaces/implementation_spec.rb | 30 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java b/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java index b4ad28fac29..a491a7b6337 100644 --- a/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java +++ b/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java @@ -201,6 +201,7 @@ else if ( cType.isPrimitive() && msType.isAssignableFrom(getBoxType(cType)) ) { // java.io.File#listFiles(java.io.FileFilter) ... accept(File) // java.io.File#listFiles(java.io.FilenameFilter) ... accept(File, String) int procArity = ((RubyProc) lastArg).getBlock().arity().getValue(); + if (procArity < 0) procArity = - (procArity + 1); /* fixedArity = false; */ final Method implMethod; final int cLast = cTypes.length - 1; diff --git a/spec/java_integration/interfaces/implementation_spec.rb b/spec/java_integration/interfaces/implementation_spec.rb index e3f54a0341f..ae0d2349460 100644 --- a/spec/java_integration/interfaces/implementation_spec.rb +++ b/spec/java_integration/interfaces/implementation_spec.rb @@ -224,6 +224,36 @@ def callIt dir.should be_kind_of(java.io.File) name.should be_kind_of(String) end + + java.io.File.new('.').listFiles do |pathname, *args| + pathname.should be_kind_of(java.io.File) + args.should be_empty + end + + java.io.File.new('.').listFiles do |dir, name, *args| + dir.should be_kind_of(java.io.File) + name.should be_kind_of(String) + args.should be_empty + end + + java.io.File.new('.').listFiles do |*args| + args[0].should be_kind_of(java.io.File) + args.size.should eql 1 + end + + #executor = java.util.concurrent.Executors.newSingleThreadExecutor + #executor.execute { |*args| args.should be_empty }; sleep 0.1 + #executor.shutdown + + work_queue = java.util.concurrent.LinkedBlockingQueue.new + executor = java.util.concurrent.ThreadPoolExecutor.new(0, 2, 0, java.util.concurrent.TimeUnit::SECONDS, work_queue) do + |*args| # newThread(Runnable) + args[0].should be_kind_of(java.lang.Runnable) + args.size.should eql 1 + java.lang.Thread.new(args[0]) + end + executor.execute { |*args| args.should be_empty }; sleep 0.1 + executor.shutdown end it "should maintain Ruby object equality when passed through Java and back" do From 9599674d7286d0ec89d7483c6081acb82033b877 Mon Sep 17 00:00:00 2001 From: kares Date: Mon, 20 Jul 2015 09:13:57 +0200 Subject: [PATCH 16/40] [ji] fix missing local assing + do not match duckables when primitivable/assignable found this avoids an incorrect ambiguous warning when matching proc-to-iface methods e.g. : `executor_spec.rb:22 warning: ambiguous Java methods found, using submit(java.util.concurrent.Callable)` ... also exploded the 3 level nested for loop to be only 2 levels --- .../jruby/java/dispatch/CallableSelector.java | 53 +++++++++++++------ 1 file changed, 37 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java b/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java index a491a7b6337..9dcd282b06e 100644 --- a/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java +++ b/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java @@ -176,8 +176,7 @@ private static T findMatchingCallableForArgs(final Ru for ( int i = 0; i < msTypes.length; i++ ) { final Class msType = msTypes[i], cType = cTypes[i]; if ( msType != cType && msType.isAssignableFrom(cType) ) { - mostSpecific = candidate; - msTypes = cTypes; + mostSpecific = candidate; msTypes = cTypes; ambiguous = false; continue OUTER; } } @@ -213,21 +212,21 @@ else if ( cType.isPrimitive() && msType.isAssignableFrom(getBoxType(cType)) ) { if ( mostSpecificArity == methodArity ) ambiguous = true; // 2 with same arity // TODO we could try to match parameter types with arg types else { - mostSpecific = candidate; ambiguous = false; - mostSpecificArity = procArity; + mostSpecific = candidate; msTypes = cTypes; + mostSpecificArity = procArity; ambiguous = false; } - continue /* OUTER */; // we want to check all + continue; /* OUTER */ // we want to check all } else if ( methodArity < procArity && mostSpecificArity != procArity ) { // not ideal but still usable if ( mostSpecificArity == methodArity ) ambiguous = true; // 2 with same arity else if ( mostSpecificArity < methodArity ) { // candidate is "better" match - mostSpecific = candidate; ambiguous = false; - mostSpecificArity = methodArity; + mostSpecific = candidate; msTypes = cTypes; + mostSpecificArity = methodArity; ambiguous = false; } - continue /* OUTER */; + continue; /* OUTER */ } else { // we're not a match and if there's something else matched than it's not really ambiguous - ambiguous = false; /* continue /* OUTER */; + ambiguous = false; /* continue; /* OUTER */ } } } @@ -309,17 +308,42 @@ private static List findCallableCandidates(final T ParameterTypes[] incoming = callables.clone(); for ( int i = 0; i < args.length; i++ ) { - retained.clear(); - for ( final Matcher matcher : NON_EXACT_MATCH_SEQUENCE ) { + retained.clear(); // non-exact match sequence : + // PRIMITIVABLE : + for ( int c = 0; c < incoming.length; c++ ) { + ParameterTypes callable = incoming[c]; + if ( callable == null ) continue; // removed (matched) + + Class[] types = callable.getParameterTypes(); + + if ( PRIMITIVABLE.match( types[i], args[i] ) ) { + retained.add((T) callable); + incoming[c] = null; // retaining - remove + } + } + // ASSIGNABLE : + for ( int c = 0; c < incoming.length; c++ ) { + ParameterTypes callable = incoming[c]; + if ( callable == null ) continue; // removed (matched) + + Class[] types = callable.getParameterTypes(); + + if ( ASSIGNABLE.match( types[i], args[i] ) ) { + retained.add((T) callable); + incoming[c] = null; // retaining - remove + } + } + if ( retained.isEmpty() ) { + // DUCKABLE : for ( int c = 0; c < incoming.length; c++ ) { ParameterTypes callable = incoming[c]; if ( callable == null ) continue; // removed (matched) Class[] types = callable.getParameterTypes(); - if ( matcher.match( types[i], args[i] ) ) { + if ( DUCKABLE.match( types[i], args[i] ) ) { retained.add((T) callable); - incoming[c] = null; // retaining - remove + //incoming[c] = null; // retaining - remove } } } @@ -417,9 +441,6 @@ public boolean match(Class type, IRubyObject arg) { @Override public String toString() { return "DUCKABLE"; } // for debugging }; - //private static final Matcher[] MATCH_SEQUENCE = new Matcher[] { EXACT, PRIMITIVABLE, ASSIGNABLE, DUCKABLE }; - private static final Matcher[] NON_EXACT_MATCH_SEQUENCE = new Matcher[] { PRIMITIVABLE, ASSIGNABLE, DUCKABLE }; - private static boolean exactMatch(ParameterTypes paramTypes, IRubyObject... args) { Class[] types = paramTypes.getParameterTypes(); From 120e0ab03bf9b9cf4b255ce24d34ab0f36f0be2f Mon Sep 17 00:00:00 2001 From: kares Date: Mon, 20 Jul 2015 14:07:38 +0200 Subject: [PATCH 17/40] forgot to initialize the mostSpecificArity for the first list element (thus not matching) --- .../jruby/java/dispatch/CallableSelector.java | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java b/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java index 9dcd282b06e..7d629900779 100644 --- a/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java +++ b/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java @@ -169,6 +169,19 @@ private static T findMatchingCallableForArgs(final Ru final IRubyObject lastArg = args.length > 0 ? args[ args.length - 1 ] : null; int mostSpecificArity = Integer.MIN_VALUE; + final int procArity; + if ( lastArg instanceof RubyProc ) { + final Method implMethod; final int last = msTypes.length - 1; + if ( last >= 0 && msTypes[last].isInterface() && ( implMethod = getFunctionalInterfaceMethod(msTypes[last]) ) != null ) { + mostSpecificArity = implMethod.getParameterTypes().length; + } + int arity = ((RubyProc) lastArg).getBlock().arity().getValue(); + procArity = arity >= 0 ? arity : - (arity + 1); /* fixedArity = false; */; + } + else { + procArity = Integer.MIN_VALUE; + } + OUTER: for ( int c = 1; c < size; c++ ) { final T candidate = candidates.get(c); final Class[] cTypes = candidate.getParameterTypes(); @@ -195,16 +208,12 @@ else if ( cType.isPrimitive() && msType.isAssignableFrom(getBoxType(cType)) ) { } // special handling if we're dealing with Proc#impl : - if ( lastArg instanceof RubyProc ) { + if ( procArity != Integer.MIN_VALUE ) { // lastArg instanceof RubyProc // cases such as (both ifaces - differ in arg count) : // java.io.File#listFiles(java.io.FileFilter) ... accept(File) // java.io.File#listFiles(java.io.FilenameFilter) ... accept(File, String) - int procArity = ((RubyProc) lastArg).getBlock().arity().getValue(); - if (procArity < 0) procArity = - (procArity + 1); /* fixedArity = false; */ - - final Method implMethod; final int cLast = cTypes.length - 1; - - if ( cLast >= 0 && cTypes[cLast].isInterface() && ( implMethod = getFunctionalInterfaceMethod(cTypes[cLast]) ) != null ) { + final Method implMethod; final int last = cTypes.length - 1; + if ( last >= 0 && cTypes[last].isInterface() && ( implMethod = getFunctionalInterfaceMethod(cTypes[last]) ) != null ) { // we're sure to have an interface in the end - match arg count : // NOTE: implMethod.getParameterCount() on Java 8 would do ... final int methodArity = implMethod.getParameterTypes().length; From 623a6f1f27763e03c38129cd1ec159f39acce006 Mon Sep 17 00:00:00 2001 From: kares Date: Mon, 20 Jul 2015 14:12:02 +0200 Subject: [PATCH 18/40] a unit test since we can not influence getMethods order (closer to failure at travis-ci) --- .../java/dispatch/CallableSelectorTest.java | 105 ++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 core/src/test/java/org/jruby/java/dispatch/CallableSelectorTest.java diff --git a/core/src/test/java/org/jruby/java/dispatch/CallableSelectorTest.java b/core/src/test/java/org/jruby/java/dispatch/CallableSelectorTest.java new file mode 100644 index 00000000000..4c382903501 --- /dev/null +++ b/core/src/test/java/org/jruby/java/dispatch/CallableSelectorTest.java @@ -0,0 +1,105 @@ +/* + * Copyright (c) 2015 JRuby. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + */ +package org.jruby.java.dispatch; + +import java.lang.reflect.Method; +import org.jruby.Ruby; +import org.jruby.RubyProc; +import org.jruby.javasupport.JavaMethod; +import org.jruby.runtime.Arity; +import org.jruby.runtime.Binding; +import org.jruby.runtime.Block; +import org.jruby.runtime.BlockBody; +import org.jruby.runtime.Frame; +import org.jruby.runtime.NullBlockBody; +import org.jruby.runtime.backtrace.BacktraceElement; +import org.jruby.runtime.builtin.IRubyObject; +import org.jruby.util.collections.IntHashMap; + +import org.junit.Test; +import static org.junit.Assert.*; + +/** + * @author kares + */ +public class CallableSelectorTest { + + @Test + public void testCallableProcToIfaceMatchIsNotOrderSensitive() throws Exception { + final Ruby runtime = Ruby.newInstance(); + + final Method list1 = java.io.File.class.getMethod("listFiles", java.io.FileFilter.class); + final Method list2 = java.io.File.class.getMethod("listFiles", java.io.FilenameFilter.class); + + IntHashMap cache; + JavaMethod[] methods; + Binding binding = new Binding(new Frame(), null, null, new BacktraceElement()); + BlockBody body1 = new NullBlockBody() { + @Override public Arity arity() { return Arity.ONE_ARGUMENT; } + }; + RubyProc dummyProc = RubyProc.newProc(runtime, new Block(body1, binding), Block.Type.PROC); + + JavaMethod result; IRubyObject[] args; + + cache = IntHashMap.nullMap(); + methods = new JavaMethod[] { + new JavaMethod(runtime, list1), new JavaMethod(runtime, list2) + }; + result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc); + assertEquals(new JavaMethod(runtime, list1), result); + + cache = IntHashMap.nullMap(); + args = new IRubyObject[] { dummyProc }; + result = CallableSelector.matchingCallableArityN(runtime, cache, methods, args); + assertEquals(new JavaMethod(runtime, list1), result); + + cache = IntHashMap.nullMap(); + methods = new JavaMethod[] { + new JavaMethod(runtime, list2), new JavaMethod(runtime, list1) + }; + result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc); + assertEquals(new JavaMethod(runtime, list1), result); + + cache = IntHashMap.nullMap(); + args = new IRubyObject[] { dummyProc }; + result = CallableSelector.matchingCallableArityN(runtime, cache, methods, args); + assertEquals(new JavaMethod(runtime, list1), result); + + // + + BlockBody body2 = new NullBlockBody() { + @Override public Arity arity() { return Arity.TWO_ARGUMENTS; } + }; + dummyProc = RubyProc.newProc(runtime, new Block(body2, binding), Block.Type.PROC); + + cache = IntHashMap.nullMap(); + methods = new JavaMethod[] { + new JavaMethod(runtime, list1), new JavaMethod(runtime, list2) + }; + result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc); + assertEquals(new JavaMethod(runtime, list2), result); + + cache = IntHashMap.nullMap(); + args = new IRubyObject[] { dummyProc }; + result = CallableSelector.matchingCallableArityN(runtime, cache, methods, args); + assertEquals(new JavaMethod(runtime, list2), result); + + cache = IntHashMap.nullMap(); + methods = new JavaMethod[] { + new JavaMethod(runtime, list2), new JavaMethod(runtime, list1) + }; + result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc); + assertEquals(new JavaMethod(runtime, list2), result); + + cache = IntHashMap.nullMap(); + args = new IRubyObject[] { dummyProc }; + result = CallableSelector.matchingCallableArityN(runtime, cache, methods, args); + assertEquals(new JavaMethod(runtime, list2), result); + } + +} From 3d7ac8fe2e6073a162d73e5db829aa1868a41287 Mon Sep 17 00:00:00 2001 From: kares Date: Tue, 21 Jul 2015 09:20:10 +0200 Subject: [PATCH 19/40] [ji] travis showcased negative proc-to-iface arity (still) dependent on getMethods order --- .../jruby/java/dispatch/CallableSelector.java | 11 ++++-- .../java/dispatch/CallableSelectorTest.java | 34 ++++++++++++++++--- 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java b/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java index 7d629900779..c710928ba5c 100644 --- a/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java +++ b/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java @@ -175,8 +175,7 @@ private static T findMatchingCallableForArgs(final Ru if ( last >= 0 && msTypes[last].isInterface() && ( implMethod = getFunctionalInterfaceMethod(msTypes[last]) ) != null ) { mostSpecificArity = implMethod.getParameterTypes().length; } - int arity = ((RubyProc) lastArg).getBlock().arity().getValue(); - procArity = arity >= 0 ? arity : - (arity + 1); /* fixedArity = false; */; + procArity = ((RubyProc) lastArg).getBlock().arity().getValue(); } else { procArity = Integer.MIN_VALUE; @@ -234,6 +233,14 @@ else if ( mostSpecificArity < methodArity ) { // candidate is "better" match } continue; /* OUTER */ } + else if ( procArity < 0 && methodArity >= -(procArity + 1) && mostSpecificArity != procArity ) { + if ( mostSpecificArity == methodArity ) ambiguous = true; // 2 with same arity + else if ( mostSpecificArity > methodArity ) { // candidate is "better" match + mostSpecific = candidate; msTypes = cTypes; + mostSpecificArity = methodArity; ambiguous = false; + } + continue; /* OUTER */ + } else { // we're not a match and if there's something else matched than it's not really ambiguous ambiguous = false; /* continue; /* OUTER */ } diff --git a/core/src/test/java/org/jruby/java/dispatch/CallableSelectorTest.java b/core/src/test/java/org/jruby/java/dispatch/CallableSelectorTest.java index 4c382903501..46a5de6a185 100644 --- a/core/src/test/java/org/jruby/java/dispatch/CallableSelectorTest.java +++ b/core/src/test/java/org/jruby/java/dispatch/CallableSelectorTest.java @@ -39,13 +39,15 @@ public void testCallableProcToIfaceMatchIsNotOrderSensitive() throws Exception { IntHashMap cache; JavaMethod[] methods; Binding binding = new Binding(new Frame(), null, null, new BacktraceElement()); + JavaMethod result; IRubyObject[] args; + + // arity 1 : + BlockBody body1 = new NullBlockBody() { @Override public Arity arity() { return Arity.ONE_ARGUMENT; } }; RubyProc dummyProc = RubyProc.newProc(runtime, new Block(body1, binding), Block.Type.PROC); - JavaMethod result; IRubyObject[] args; - cache = IntHashMap.nullMap(); methods = new JavaMethod[] { new JavaMethod(runtime, list1), new JavaMethod(runtime, list2) @@ -59,7 +61,7 @@ public void testCallableProcToIfaceMatchIsNotOrderSensitive() throws Exception { assertEquals(new JavaMethod(runtime, list1), result); cache = IntHashMap.nullMap(); - methods = new JavaMethod[] { + methods = new JavaMethod[] { // "reverse" method order new JavaMethod(runtime, list2), new JavaMethod(runtime, list1) }; result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc); @@ -70,7 +72,7 @@ public void testCallableProcToIfaceMatchIsNotOrderSensitive() throws Exception { result = CallableSelector.matchingCallableArityN(runtime, cache, methods, args); assertEquals(new JavaMethod(runtime, list1), result); - // + // arity 2 : BlockBody body2 = new NullBlockBody() { @Override public Arity arity() { return Arity.TWO_ARGUMENTS; } @@ -90,7 +92,7 @@ public void testCallableProcToIfaceMatchIsNotOrderSensitive() throws Exception { assertEquals(new JavaMethod(runtime, list2), result); cache = IntHashMap.nullMap(); - methods = new JavaMethod[] { + methods = new JavaMethod[] { // "reverse" method order new JavaMethod(runtime, list2), new JavaMethod(runtime, list1) }; result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc); @@ -100,6 +102,28 @@ public void testCallableProcToIfaceMatchIsNotOrderSensitive() throws Exception { args = new IRubyObject[] { dummyProc }; result = CallableSelector.matchingCallableArityN(runtime, cache, methods, args); assertEquals(new JavaMethod(runtime, list2), result); + + // arity -1 : + + BlockBody body_1 = new NullBlockBody() { // arity -1 + @Override public Arity arity() { return Arity.OPTIONAL; } + }; + dummyProc = RubyProc.newProc(runtime, new Block(body_1, binding), Block.Type.PROC); + + cache = IntHashMap.nullMap(); + methods = new JavaMethod[] { + new JavaMethod(runtime, list1), new JavaMethod(runtime, list2) + }; + result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc); + assertEquals(new JavaMethod(runtime, list1), result); + + cache = IntHashMap.nullMap(); + methods = new JavaMethod[] { + new JavaMethod(runtime, list2), new JavaMethod(runtime, list1) + }; + result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc); + assertEquals(new JavaMethod(runtime, list1), result); + } } From e20d6c7b0db87c6651dc71a0cbb0be093c3997ec Mon Sep 17 00:00:00 2001 From: kares Date: Tue, 21 Jul 2015 15:30:33 +0200 Subject: [PATCH 20/40] travis showcased we did not handle all negative proc-to-iface arity cases correctly ... they were still dependent on getMethods order (travis-ci failures in **spec:ji**) --- .../jruby/java/dispatch/CallableSelector.java | 35 ++++++++++------ .../java/dispatch/CallableSelectorTest.java | 42 +++++++++++++++++++ 2 files changed, 64 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java b/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java index c710928ba5c..fd0b974942b 100644 --- a/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java +++ b/core/src/main/java/org/jruby/java/dispatch/CallableSelector.java @@ -225,21 +225,30 @@ else if ( cType.isPrimitive() && msType.isAssignableFrom(getBoxType(cType)) ) { } continue; /* OUTER */ // we want to check all } - else if ( methodArity < procArity && mostSpecificArity != procArity ) { // not ideal but still usable - if ( mostSpecificArity == methodArity ) ambiguous = true; // 2 with same arity - else if ( mostSpecificArity < methodArity ) { // candidate is "better" match - mostSpecific = candidate; msTypes = cTypes; - mostSpecificArity = methodArity; ambiguous = false; + else if ( mostSpecificArity != procArity ) { + if ( methodArity < procArity ) { // not ideal but still usable + if ( mostSpecificArity == methodArity ) ambiguous = true; // 2 with same arity + else if ( mostSpecificArity < methodArity ) { // candidate is "better" match + mostSpecific = candidate; msTypes = cTypes; + mostSpecificArity = methodArity; ambiguous = false; + } + continue; /* OUTER */ } - continue; /* OUTER */ - } - else if ( procArity < 0 && methodArity >= -(procArity + 1) && mostSpecificArity != procArity ) { - if ( mostSpecificArity == methodArity ) ambiguous = true; // 2 with same arity - else if ( mostSpecificArity > methodArity ) { // candidate is "better" match - mostSpecific = candidate; msTypes = cTypes; - mostSpecificArity = methodArity; ambiguous = false; + else if ( procArity < 0 && methodArity >= -(procArity + 1) ) { // *splat that fits + if ( mostSpecificArity == methodArity ) ambiguous = true; // 2 with same arity + else { + final int msa = mostSpecificArity + procArity; + final int ma = methodArity + procArity; + if ( ( msa < 0 && ma < 0 && msa < ma ) || + ( msa >= 0 && ma >= 0 && msa > ma ) || + ( msa > ma ) ) { + mostSpecific = candidate; msTypes = cTypes; + mostSpecificArity = methodArity; // ambiguous = false; + } + } + ambiguous = false; + continue; /* OUTER */ } - continue; /* OUTER */ } else { // we're not a match and if there's something else matched than it's not really ambiguous ambiguous = false; /* continue; /* OUTER */ diff --git a/core/src/test/java/org/jruby/java/dispatch/CallableSelectorTest.java b/core/src/test/java/org/jruby/java/dispatch/CallableSelectorTest.java index 46a5de6a185..aad34cb80a4 100644 --- a/core/src/test/java/org/jruby/java/dispatch/CallableSelectorTest.java +++ b/core/src/test/java/org/jruby/java/dispatch/CallableSelectorTest.java @@ -124,6 +124,48 @@ public void testCallableProcToIfaceMatchIsNotOrderSensitive() throws Exception { result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc); assertEquals(new JavaMethod(runtime, list1), result); + // arity -3 : + + BlockBody body_3 = new NullBlockBody() { // arity -3 + @Override public Arity arity() { return Arity.TWO_REQUIRED; } + }; + dummyProc = RubyProc.newProc(runtime, new Block(body_3, binding), Block.Type.PROC); + + cache = IntHashMap.nullMap(); + methods = new JavaMethod[] { + new JavaMethod(runtime, list1), new JavaMethod(runtime, list2) + }; + result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc); + assertEquals(new JavaMethod(runtime, list2), result); + + cache = IntHashMap.nullMap(); + methods = new JavaMethod[] { + new JavaMethod(runtime, list2), new JavaMethod(runtime, list1) + }; + result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc); + assertEquals(new JavaMethod(runtime, list2), result); + + // arity -2 : + + BlockBody body_2 = new NullBlockBody() { // arity -2 (arg1, *rest) should prefer (single) + @Override public Arity arity() { return Arity.ONE_REQUIRED; } + }; + dummyProc = RubyProc.newProc(runtime, new Block(body_2, binding), Block.Type.PROC); + + cache = IntHashMap.nullMap(); + methods = new JavaMethod[] { + new JavaMethod(runtime, list1), new JavaMethod(runtime, list2) + }; + result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc); + assertEquals(new JavaMethod(runtime, list1), result); + + cache = IntHashMap.nullMap(); + methods = new JavaMethod[] { + new JavaMethod(runtime, list2), new JavaMethod(runtime, list1) + }; + result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc); + assertEquals(new JavaMethod(runtime, list1), result); + } } From a1a99aaf88c0e1bdbf7e6a3624da8bedc5dc319e Mon Sep 17 00:00:00 2001 From: kares Date: Tue, 21 Jul 2015 19:10:27 +0200 Subject: [PATCH 21/40] remove some import jul left-overs + finalize accessibleObject() on JavaMethod --- core/src/main/java/org/jruby/javasupport/JavaMethod.java | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/jruby/javasupport/JavaMethod.java b/core/src/main/java/org/jruby/javasupport/JavaMethod.java index 61257453c22..7d12df21fa5 100644 --- a/core/src/main/java/org/jruby/javasupport/JavaMethod.java +++ b/core/src/main/java/org/jruby/javasupport/JavaMethod.java @@ -46,7 +46,6 @@ import org.jruby.Ruby; import org.jruby.RubyBoolean; import org.jruby.RubyClass; -import org.jruby.RubyInstanceConfig; import org.jruby.RubyModule; import org.jruby.RubyString; import org.jruby.anno.JRubyClass; @@ -57,8 +56,6 @@ import org.jruby.javasupport.proxy.JavaProxyMethod; import org.jruby.runtime.ObjectAllocator; import org.jruby.runtime.builtin.IRubyObject; -import org.jruby.util.log.Logger; -import org.jruby.util.log.LoggerFactory; import static org.jruby.util.CodegenUtils.getBoxType; import static org.jruby.util.CodegenUtils.prettyParams; @@ -66,8 +63,6 @@ @JRubyClass(name="Java::JavaMethod") public class JavaMethod extends JavaCallable { - //private static final Logger LOG = LoggerFactory.getLogger("JavaMethod"); - //private final static boolean USE_HANDLES = RubyInstanceConfig.USE_GENERATED_HANDLES; //private final static boolean HANDLE_DEBUG = false; @@ -188,7 +183,7 @@ public static JavaMethod getMatchingDeclaredMethod(Ruby runtime, Class javaCl @Override public final boolean equals(Object other) { - return other instanceof JavaMethod && this.method.equals( ((JavaMethod)other).method ); + return other instanceof JavaMethod && this.method.equals( ((JavaMethod) other).method ); } @Override @@ -594,7 +589,7 @@ public String toGenericString() { } @Override - public AccessibleObject accessibleObject() { + public final AccessibleObject accessibleObject() { return method; } From 13a5e4150b98611e3942c075d29c4a777782fd1c Mon Sep 17 00:00:00 2001 From: Christian Meier Date: Thu, 23 Jul 2015 20:25:50 +0200 Subject: [PATCH 22/40] have no side effects when creating instance of IsolatedScriptingContainer backport and improve the implementation from 9k. also sets the current directory to uri:classloader:/ if this directory exists. same with jruby.home which will only be set into the classloader when it exists. Sponsored by Lookout Inc. --- .../embed/IsolatedScriptingContainer.java | 33 ++++++++++++------- lib/ruby/shared/rubygems/defaults/jruby.rb | 8 +++-- .../embed/osgi/test/JRubyOsgiEmbedTest.java | 16 ++++----- .../embed/osgi/test/JRubyOsgiEmbedTest.java | 4 ++- 4 files changed, 39 insertions(+), 22 deletions(-) diff --git a/core/src/main/java/org/jruby/embed/IsolatedScriptingContainer.java b/core/src/main/java/org/jruby/embed/IsolatedScriptingContainer.java index c11ad250264..cbcfab93b42 100644 --- a/core/src/main/java/org/jruby/embed/IsolatedScriptingContainer.java +++ b/core/src/main/java/org/jruby/embed/IsolatedScriptingContainer.java @@ -34,6 +34,7 @@ */ public class IsolatedScriptingContainer extends ScriptingContainer { + private static final String URI_CLASSLOADER = "uri:classloader:/"; private static final String JRUBYDIR = "/.jrubydir"; private static final String JRUBY_HOME = "/META-INF/jruby.home"; @@ -69,16 +70,18 @@ public IsolatedScriptingContainer( LocalContextScope scope, if (cl == null) cl = Thread.currentThread().getContextClassLoader(); setClassLoader( cl ); - setLoadPaths( Arrays.asList( "uri:classloader:" ) ); + setLoadPaths( Arrays.asList( URI_CLASSLOADER ) ); // set the right jruby home - setHomeDirectory( "uri:classloader:" + JRUBY_HOME ); + URL url = getResource(cl, JRUBY_HOME + JRUBYDIR); + if (url != null){ + setHomeDirectory( URI_CLASSLOADER + JRUBY_HOME ); + } - // setup the isolated GEM_PATH, i.e. without $HOME/.gem/** - runScriptlet("require 'rubygems/defaults/jruby';" - + "Gem::Specification.reset;" - + "Gem::Specification.add_dir 'uri:classloader:" + JRUBY_HOME + "/lib/ruby/gems/shared';" - + "Gem::Specification.add_dir 'uri:classloader:';"); + url = getResource(cl, JRUBYDIR); + if (url != null){ + setCurrentDirectory( URI_CLASSLOADER ); + } // setup the isolated GEM_PATH, i.e. without $HOME/.gem/** setEnvironment(null); @@ -87,11 +90,11 @@ public IsolatedScriptingContainer( LocalContextScope scope, @Override public void setEnvironment(Map environment) { if (environment == null || !environment.containsKey("GEM_PATH") - || !environment.containsKey("GEM_HOME")|| !environment.containsKey("JARS_HOME")) { + || !environment.containsKey("GEM_HOME") || !environment.containsKey("JARS_HOME")) { Map env = environment == null ? new HashMap() : new HashMap(environment); - if (!env.containsKey("GEM_PATH")) env.put("GEM_PATH", "uri:classloader://"); - if (!env.containsKey("GEM_HOME")) env.put("GEM_HOME", "uri:classloader://"); - if (!env.containsKey("JARS_HOME")) env.put("JARS_HOME", "uri:classloader://jars"); + if (!env.containsKey("GEM_PATH")) env.put("GEM_PATH", URI_CLASSLOADER); + if (!env.containsKey("GEM_HOME")) env.put("GEM_HOME", URI_CLASSLOADER); + if (!env.containsKey("JARS_HOME")) env.put("JARS_HOME", URI_CLASSLOADER + "jars"); super.setEnvironment(env); } else { @@ -130,6 +133,14 @@ private String createUri(ClassLoader cl, String ref) { return "uri:" + url.toString().replaceFirst( ref + "$", "" ); } + private URL getResource(ClassLoader cl, String ref) { + URL url = cl.getResource( ref ); + if ( url == null && ref.startsWith( "/" ) ) { + url = cl.getResource( ref.substring( 1 ) ); + } + return url; + } + protected void addGemPath(String uri) { runScriptlet( "Gem::Specification.add_dir '" + uri + "' unless Gem::Specification.dirs.member?( '" + uri + "' )" ); } diff --git a/lib/ruby/shared/rubygems/defaults/jruby.rb b/lib/ruby/shared/rubygems/defaults/jruby.rb index 33083b04426..4e973bf62ca 100644 --- a/lib/ruby/shared/rubygems/defaults/jruby.rb +++ b/lib/ruby/shared/rubygems/defaults/jruby.rb @@ -85,7 +85,7 @@ def dirs @@dirs ||= Gem.path.collect {|dir| if File.file?(dir) && dir =~ /\.jar$/ "file:#{dir}!/specifications" - elsif File.directory?(dir) || dir =~ /^file:/ + elsif File.directory?(File.join(dir, "specifications")) || dir =~ /^file:/ File.join(dir, "specifications") end }.compact + spec_directories_from_classpath @@ -108,7 +108,11 @@ def dirs= dirs end def spec_directories_from_classpath - stuff = JRuby::Util.classloader_resources("specifications") + stuff = [ 'uri:classloader://specifications' ] + JRuby::Util.classloader_resources("specifications") + # some classloader return directory info. use only the "protocols" + # which jruby understands + stuff.select! { |s| File.directory?( s ) } + stuff end end end diff --git a/maven/jruby-complete/src/templates/osgi_many_bundles_with_embedded_gems/test/src/test/java/org/jruby/embed/osgi/test/JRubyOsgiEmbedTest.java b/maven/jruby-complete/src/templates/osgi_many_bundles_with_embedded_gems/test/src/test/java/org/jruby/embed/osgi/test/JRubyOsgiEmbedTest.java index 71710421dd8..873d5187897 100644 --- a/maven/jruby-complete/src/templates/osgi_many_bundles_with_embedded_gems/test/src/test/java/org/jruby/embed/osgi/test/JRubyOsgiEmbedTest.java +++ b/maven/jruby-complete/src/templates/osgi_many_bundles_with_embedded_gems/test/src/test/java/org/jruby/embed/osgi/test/JRubyOsgiEmbedTest.java @@ -77,13 +77,13 @@ public void testJRubyCreate() throws Exception { System.err.println(); System.err.println(); - // System.setProperty( "jruby.debug.loadService", "true" ); - //System.setProperty( "jruby.native.enabled", "true" ); + //System.setProperty( "jruby.debug.loadService", "true" ); + //System.setProperty( "jruby.native.enabled", "true" ); OSGiIsolatedScriptingContainer jruby = new OSGiIsolatedScriptingContainer(); - jruby.addBundleToLoadPath( "org.jruby.osgi.scripts-bundle" ); - jruby.addBundleToGemPath( FrameworkUtil.getBundle( Gems.class ) ); - + jruby.addBundleToLoadPath( "org.jruby.osgi.scripts-bundle" ); + jruby.addBundleToGemPath( FrameworkUtil.getBundle( Gems.class ) ); + // run a script from LOAD_PATH String hello = (String) jruby.runScriptlet( "require 'hello'; Hello.say" ); assertEquals( hello, "world" ); @@ -93,7 +93,7 @@ public void testJRubyCreate() throws Exception { String gemPath = (String) jruby.runScriptlet( "Gem::Specification.dirs.inspect" ); gemPath = gemPath.replaceAll( "bundle[^:]*://[^/]*", "bundle:/" ); - assertEquals( gemPath, "[\"uri:bundle://specifications\", \"uri:classloader:/specifications\", \"uri:classloader:/META-INF/jruby.home/lib/ruby/gems/shared/specifications\"]" ); + assertEquals( gemPath, "[\"uri:bundle://specifications\"]" ); // ensure we can load rake from the default gems boolean loaded = (Boolean) jruby.runScriptlet( "require 'rake'" ); @@ -110,7 +110,7 @@ public void testJRubyCreate() throws Exception { loaded = (Boolean) jruby.runScriptlet( "require 'openssl'" ); assertEquals(true, loaded); - jruby.runScriptlet( "require 'jar-dependencies'" ); + jruby.runScriptlet( "require 'jar-dependencies'" ); list = (String) jruby.runScriptlet( "Gem.loaded_specs.keys.inspect" ); assertEquals(list, "[\"rake\", \"jruby-openssl\", \"jar-dependencies\"]"); @@ -118,7 +118,7 @@ public void testJRubyCreate() throws Exception { loaded = (Boolean) jruby.runScriptlet( "require 'virtus'" ); assertEquals(true, loaded); - list = (String) jruby.runScriptlet( "Gem.loaded_specs.keys.inspect" ); + list = (String) jruby.runScriptlet( "Gem.loaded_specs.keys.inspect" ); assertEquals(list, "[\"rake\", \"jruby-openssl\", \"jar-dependencies\", \"thread_safe\", \"descendants_tracker\", \"equalizer\", \"coercible\", \"ice_nine\", \"axiom-types\", \"virtus\"]"); } } diff --git a/maven/jruby/src/templates/osgi_all_inclusive/src/test/java/org/jruby/embed/osgi/test/JRubyOsgiEmbedTest.java b/maven/jruby/src/templates/osgi_all_inclusive/src/test/java/org/jruby/embed/osgi/test/JRubyOsgiEmbedTest.java index 0f5510790bc..0eae6d11870 100644 --- a/maven/jruby/src/templates/osgi_all_inclusive/src/test/java/org/jruby/embed/osgi/test/JRubyOsgiEmbedTest.java +++ b/maven/jruby/src/templates/osgi_all_inclusive/src/test/java/org/jruby/embed/osgi/test/JRubyOsgiEmbedTest.java @@ -92,8 +92,10 @@ public void testJRubyCreate() throws InterruptedException { assertEquals(true, loaded); String gemPath = (String) jruby.runScriptlet( "Gem::Specification.dirs.inspect" ); + gemPath = gemPath.replaceAll( "bundle[^:]*://[^/]*", "bundle:/" ); - assertEquals( gemPath, "[\"uri:classloader:/specifications\", \"uri:classloader:/META-INF/jruby.home/lib/ruby/gems/shared/specifications\"]" ); + // TODO fix the URLResource to produce uri:classloader:// urls only + assertEquals( gemPath, "[\"uri:classloader:/specifications\", \"uri:classloader://specifications\"]" ); jruby.runScriptlet( "require 'jar-dependencies'" ); list = (String) jruby.runScriptlet( "Gem.loaded_specs.keys.inspect" ); From 814124b6e2e2befe0a0a7b5aeef704bd6638f1df Mon Sep 17 00:00:00 2001 From: Christian Meier Date: Thu, 23 Jul 2015 22:12:45 +0200 Subject: [PATCH 23/40] extracted the classloader to uri-like path into helper class and use it to provide the add classloader to LOAD_PATH or GEM_PATH feature for JavaEmbedUtils --- .../embed/IsolatedScriptingContainer.java | 46 +++++-------- .../org/jruby/javasupport/JavaEmbedUtils.java | 18 ++++- .../org/jruby/util/UriLikePathHelper.java | 62 +++++++++++++++++ .../jruby/javasupport/JavaEmbedUtilsTest.java | 66 +++++++++++++++++++ .../test/resources/java_embed_utils/.jrubydir | 0 .../resources/java_embed_utils/test_me.rb | 1 + 6 files changed, 163 insertions(+), 30 deletions(-) create mode 100644 core/src/main/java/org/jruby/util/UriLikePathHelper.java create mode 100644 core/src/test/java/org/jruby/javasupport/JavaEmbedUtilsTest.java create mode 100644 core/src/test/resources/java_embed_utils/.jrubydir create mode 100644 core/src/test/resources/java_embed_utils/test_me.rb diff --git a/core/src/main/java/org/jruby/embed/IsolatedScriptingContainer.java b/core/src/main/java/org/jruby/embed/IsolatedScriptingContainer.java index cbcfab93b42..068ff3958b0 100644 --- a/core/src/main/java/org/jruby/embed/IsolatedScriptingContainer.java +++ b/core/src/main/java/org/jruby/embed/IsolatedScriptingContainer.java @@ -3,8 +3,12 @@ import java.net.URL; import java.util.Arrays; import java.util.HashMap; +import java.util.LinkedList; +import java.util.List; import java.util.Map; +import org.jruby.util.UriLikePathHelper; + /** * the IsolatedScriptingContainer detects the whether it is used with * a Thread.currentThread.contextClassLoader (J2EE) or with the classloader @@ -70,15 +74,18 @@ public IsolatedScriptingContainer( LocalContextScope scope, if (cl == null) cl = Thread.currentThread().getContextClassLoader(); setClassLoader( cl ); - setLoadPaths( Arrays.asList( URI_CLASSLOADER ) ); + List loadPaths = new LinkedList(); + loadPaths.add(URI_CLASSLOADER); + setLoadPaths(loadPaths); // set the right jruby home - URL url = getResource(cl, JRUBY_HOME + JRUBYDIR); + UriLikePathHelper uriPath = new UriLikePathHelper(cl); + URL url = uriPath.getResource(JRUBY_HOME + JRUBYDIR); if (url != null){ setHomeDirectory( URI_CLASSLOADER + JRUBY_HOME ); } - url = getResource(cl, JRUBYDIR); + url = uriPath.getResource(JRUBYDIR); if (url != null){ setCurrentDirectory( URI_CLASSLOADER ); } @@ -102,43 +109,26 @@ public void setEnvironment(Map environment) { } } - public void addLoadPath( ClassLoader cl ) { - addLoadPath( cl, JRUBYDIR ); + public void addLoadPath(ClassLoader cl) { + addLoadPath(new UriLikePathHelper(cl).getUriLikePath()); } public void addLoadPath( ClassLoader cl, String ref ) { - addLoadPath(createUri(cl, ref)); + addLoadPath(new UriLikePathHelper(cl).getUriLikePath(ref)); } protected void addLoadPath(String uri) { - runScriptlet( "$LOAD_PATH << '" + uri + "' unless $LOAD_PATH.member?( '" + uri + "' )" ); + if (!getLoadPaths().contains(uri)) { + getLoadPaths().add(uri); + } } public void addGemPath( ClassLoader cl ) { - addGemPath( cl, "/specifications" + JRUBYDIR ); + addGemPath(new UriLikePathHelper(cl).getUriLikePath()); } public void addGemPath( ClassLoader cl, String ref ) { - addGemPath(createUri(cl, ref)); - } - - private String createUri(ClassLoader cl, String ref) { - URL url = cl.getResource( ref ); - if ( url == null && ref.startsWith( "/" ) ) { - url = cl.getResource( ref.substring( 1 ) ); - } - if ( url == null ) { - throw new RuntimeException( "reference " + ref + " not found on classloader " + cl ); - } - return "uri:" + url.toString().replaceFirst( ref + "$", "" ); - } - - private URL getResource(ClassLoader cl, String ref) { - URL url = cl.getResource( ref ); - if ( url == null && ref.startsWith( "/" ) ) { - url = cl.getResource( ref.substring( 1 ) ); - } - return url; + addGemPath(new UriLikePathHelper(cl).getUriLikePath(ref)); } protected void addGemPath(String uri) { diff --git a/core/src/main/java/org/jruby/javasupport/JavaEmbedUtils.java b/core/src/main/java/org/jruby/javasupport/JavaEmbedUtils.java index 1f1cb7d3a15..25bd8283d47 100644 --- a/core/src/main/java/org/jruby/javasupport/JavaEmbedUtils.java +++ b/core/src/main/java/org/jruby/javasupport/JavaEmbedUtils.java @@ -28,6 +28,7 @@ * the terms of any one of the EPL, the GPL or the LGPL. ***** END LICENSE BLOCK *****/ +import java.io.IOException; import java.io.InputStream; import java.util.List; @@ -40,10 +41,11 @@ import org.jruby.RubyRuntimeAdapter; import org.jruby.RubyString; import org.jruby.ast.Node; -import org.jruby.runtime.Helpers; import org.jruby.runtime.Block; +import org.jruby.runtime.Helpers; import org.jruby.runtime.builtin.IRubyObject; import org.jruby.util.ClassCache; +import org.jruby.util.UriLikePathHelper; /** * Utility functions to help embedders out. These function consolidate logic that is @@ -93,7 +95,6 @@ public static Ruby initialize(List loadPaths, ClassCache classCache) { public static Ruby initialize(List loadPaths, RubyInstanceConfig config) { Ruby runtime = Ruby.newInstance(config); runtime.getLoadService().addPaths(loadPaths); - runtime.getLoadService().require("java"); return runtime; } @@ -229,6 +230,15 @@ public IRubyObject run() { } } + public static void addLoadPath(Ruby runtime, ClassLoader cl) { + runtime.getLoadService().addPaths(new UriLikePathHelper(cl).getUriLikePath()); + } + + public static void addGemPath(Ruby runtime, ClassLoader cl) { + String uri = new UriLikePathHelper(cl).getUriLikePath(); + runtime.evalScriptlet("Gem::Specification.add_dir '" + uri + "' unless Gem::Specification.dirs.member?( '" + uri + "' )" ); + } + /** * Dispose of the runtime you initialized. * @@ -236,6 +246,10 @@ public IRubyObject run() { */ public static void terminate(Ruby runtime) { runtime.tearDown(); + try { + runtime.getJRubyClassLoader().close(); + } catch (IOException weTriedToCloseIt) { + } } /** diff --git a/core/src/main/java/org/jruby/util/UriLikePathHelper.java b/core/src/main/java/org/jruby/util/UriLikePathHelper.java new file mode 100644 index 00000000000..5fde4c67878 --- /dev/null +++ b/core/src/main/java/org/jruby/util/UriLikePathHelper.java @@ -0,0 +1,62 @@ +/* + **** BEGIN LICENSE BLOCK ***** + * Version: EPL 1.0/GPL 2.0/LGPL 2.1 + * + * The contents of this file are subject to the Eclipse Public + * License Version 1.0 (the "License"); you may not use this file + * except in compliance with the License. You may obtain a copy of + * the License at http://www.eclipse.org/legal/epl-v10.html + * + * Software distributed under the License is distributed on an "AS + * IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or + * implied. See the License for the specific language governing + * rights and limitations under the License. + * + * Alternatively, the contents of this file may be used under the terms of + * either of the GNU General Public License Version 2 or later (the "GPL"), + * or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), + * in which case the provisions of the GPL or the LGPL are applicable instead + * of those above. If you wish to allow use of your version of this file only + * under the terms of either the GPL or the LGPL, and not to allow others to + * use your version of this file under the terms of the EPL, indicate your + * decision by deleting the provisions above and replace them with the notice + * and other provisions required by the GPL or the LGPL. If you do not delete + * the provisions above, a recipient may use your version of this file under + * the terms of any one of the EPL, the GPL or the LGPL. + ***** END LICENSE BLOCK *****/ +package org.jruby.util; + +import java.net.URL; + +public class UriLikePathHelper { + + private final ClassLoader classLoader; + + public UriLikePathHelper(ClassLoader classLoader) { + this.classLoader = classLoader; + } + + public URL getResource(String ref) { + URL url = classLoader.getResource( ref ); + if ( url == null && ref.startsWith( "/" ) ) { + url = classLoader.getResource( ref.substring( 1 ) ); + } + return url; + } + + public String getUriLikePath() { + return createUri(".jrubydir"); + } + + public String getUriLikePath(String ref) { + return createUri(ref); + } + + String createUri(String ref) { + URL url = getResource( ref ); + if ( url == null ) { + throw new RuntimeException( "reference " + ref + " not found on classloader " + classLoader ); + } + return "uri:" + url.toString().replaceFirst( ref + "$", "" ); + } +} \ No newline at end of file diff --git a/core/src/test/java/org/jruby/javasupport/JavaEmbedUtilsTest.java b/core/src/test/java/org/jruby/javasupport/JavaEmbedUtilsTest.java new file mode 100644 index 00000000000..e414fb902cb --- /dev/null +++ b/core/src/test/java/org/jruby/javasupport/JavaEmbedUtilsTest.java @@ -0,0 +1,66 @@ +package org.jruby.javasupport; + +import java.io.File; +import java.net.URL; +import java.net.URLClassLoader; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.fail; + +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +import org.jruby.Ruby; +import org.jruby.RubyInstanceConfig; + +public class JavaEmbedUtilsTest { + + static ClassLoader cl; + + @BeforeClass + public static void setupClassLoader() { + cl = Thread.currentThread().getContextClassLoader(); + // make sure we have classloader which does not find jruby + ClassLoader c = new URLClassLoader( new URL[] {}, null ); + try { + c.loadClass( "org.jruby.embed.ScriptingContainer" ); + fail( "this classloader shall not find jruby" ); + } + catch( ClassNotFoundException expected){} + // set it as context classloader + Thread.currentThread().setContextClassLoader( c ); + } + + @AfterClass + public static void restClassLoader() { + Thread.currentThread().setContextClassLoader( cl ); + } + + private static List EMPTY = Collections.emptyList(); + + @Test + public void testAddClassloaderToLoadPathOnTCCL() throws Exception { + Thread.currentThread().setContextClassLoader( cl ); + Ruby runtime = JavaEmbedUtils.initialize(EMPTY); + URL url = new File("src/test/resources/java_embed_utils").toURI().toURL(); + JavaEmbedUtils.addLoadPath(runtime, new URLClassLoader(new URL[] { url })); + String result = runtime.evalScriptlet("require 'test_me'; $result").toString(); + assertEquals(result, "uri:" + url); + } + + @Test + public void testAddClassloaderToLoadPathOnNoneTCCL() throws Exception { + RubyInstanceConfig config = new RubyInstanceConfig(); + config.setLoader(RubyInstanceConfig.class.getClassLoader()); + Ruby runtime = JavaEmbedUtils.initialize(EMPTY, config); + URL url = new File("src/test/resources/java_embed_utils").toURI().toURL(); + JavaEmbedUtils.addLoadPath(runtime, new URLClassLoader(new URL[] { url })); + String result = runtime.evalScriptlet("require 'test_me';$result").toString(); + assertEquals(result, "uri:" + url); + } +} \ No newline at end of file diff --git a/core/src/test/resources/java_embed_utils/.jrubydir b/core/src/test/resources/java_embed_utils/.jrubydir new file mode 100644 index 00000000000..e69de29bb2d diff --git a/core/src/test/resources/java_embed_utils/test_me.rb b/core/src/test/resources/java_embed_utils/test_me.rb new file mode 100644 index 00000000000..b42e07ecb28 --- /dev/null +++ b/core/src/test/resources/java_embed_utils/test_me.rb @@ -0,0 +1 @@ +$result = $LOAD_PATH.detect { |d| d =~ /^uri:/ } From 376eba114f1e27ee56c333439f34f9b9ae5a61d9 Mon Sep 17 00:00:00 2001 From: Christian Meier Date: Fri, 24 Jul 2015 08:49:21 +0200 Subject: [PATCH 24/40] you can not close classloaders on jdk6 --- core/src/main/java/org/jruby/javasupport/JavaEmbedUtils.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/core/src/main/java/org/jruby/javasupport/JavaEmbedUtils.java b/core/src/main/java/org/jruby/javasupport/JavaEmbedUtils.java index 25bd8283d47..1d8a9246ef2 100644 --- a/core/src/main/java/org/jruby/javasupport/JavaEmbedUtils.java +++ b/core/src/main/java/org/jruby/javasupport/JavaEmbedUtils.java @@ -246,10 +246,6 @@ public static void addGemPath(Ruby runtime, ClassLoader cl) { */ public static void terminate(Ruby runtime) { runtime.tearDown(); - try { - runtime.getJRubyClassLoader().close(); - } catch (IOException weTriedToCloseIt) { - } } /** From 6ceb5374cffa0a101728cc04ccc004b1ced5f20a Mon Sep 17 00:00:00 2001 From: Christian Meier Date: Fri, 24 Jul 2015 08:54:01 +0200 Subject: [PATCH 25/40] make it 1.8 compatible --- lib/ruby/shared/rubygems/defaults/jruby.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/ruby/shared/rubygems/defaults/jruby.rb b/lib/ruby/shared/rubygems/defaults/jruby.rb index 4e973bf62ca..b4ed4d07965 100644 --- a/lib/ruby/shared/rubygems/defaults/jruby.rb +++ b/lib/ruby/shared/rubygems/defaults/jruby.rb @@ -111,8 +111,7 @@ def spec_directories_from_classpath stuff = [ 'uri:classloader://specifications' ] + JRuby::Util.classloader_resources("specifications") # some classloader return directory info. use only the "protocols" # which jruby understands - stuff.select! { |s| File.directory?( s ) } - stuff + stuff.select { |s| File.directory?( s ) } end end end From e6632c0a00fb7a43d9b8754573cb98102365cc12 Mon Sep 17 00:00:00 2001 From: Christian Meier Date: Fri, 24 Jul 2015 09:59:45 +0200 Subject: [PATCH 26/40] added nested IsolatedScriptingContainer via jruby-mains test execute code inside a jar via jruby-mains, create an IsolatedSCriptingContainer which should inherit the environment from the jruby-mains, i.e. GEM_PATH, GEM_HOME, JARS_HOME, current working directory should all point inside the jars Sponsored by Lookout Inc. --- maven/jruby/src/it/runnable/Mavenfile | 11 +++++--- maven/jruby/src/it/runnable/nested.rb | 14 ++++++++++ maven/jruby/src/it/runnable/pom.xml | 27 ++++++++++++++---- maven/jruby/src/it/runnable/spec/one_spec.rb | 2 +- maven/jruby/src/it/runnable/test.rb | 25 +++++++++++++++++ maven/jruby/src/it/runnable/test_other.rb | 29 ++++++++++++++++++++ maven/jruby/src/it/runnable/verify.bsh | 14 +++++++++- 7 files changed, 110 insertions(+), 12 deletions(-) create mode 100644 maven/jruby/src/it/runnable/nested.rb create mode 100644 maven/jruby/src/it/runnable/test.rb create mode 100644 maven/jruby/src/it/runnable/test_other.rb diff --git a/maven/jruby/src/it/runnable/Mavenfile b/maven/jruby/src/it/runnable/Mavenfile index dcd50f61321..6d23634287b 100644 --- a/maven/jruby/src/it/runnable/Mavenfile +++ b/maven/jruby/src/it/runnable/Mavenfile @@ -3,7 +3,7 @@ properties( 'tesla.dump.pom' => 'pom.xml', 'tesla.dump.readOnly' => true, 'jruby.version' => '@project.version@', - 'jruby.plugins.version' => '1.0.8' ) + 'jruby.plugins.version' => '1.0.10' ) gemfile @@ -11,13 +11,13 @@ gem 'bundler', '1.7.7' pom 'org.jruby:jruby', '${jruby.version}' -jar 'de.saumya.mojo:jruby-mains', '0.2.0' +jar 'de.saumya.mojo:jruby-mains', '0.3.1' build do directory 'pkg' end -files = [ '.rspec', 'config.ru', '*file', '*file.lock', '.jbundler/classpath.rb', +files = [ '.rspec', '*.rb', 'config.ru', '*file', '*file.lock', '.jbundler/classpath.rb', 'lib/**', 'app/**', 'config/**', 'vendor/**', 'spec/**' ] jruby_plugin!( :gem, # need a jruby-complete from maven central here @@ -40,7 +40,7 @@ if File.file?('Jarfile.lock') data = l.sub(/-\ /, '').strip.split(':') if data.size > 3 data = Hash[ [:groupId, :artifactId, :type, :version, :classifier].zip( data ) ] - data[ :outputDirectory ] = File.join( '${project.build.outputDirectory}', + data[ :outputDirectory ] = File.join( '${project.build.outputDirectory}/jars', data[:groupId].gsub(/[.]/, '/'), data[:artifactId], data[:version] ) @@ -81,6 +81,9 @@ phase :package do execute_goal( :exec, :id => 'rspec', :arguments => [ '-jar', 'runnable.jar', '-S', 'rspec' ] ) + + execute_goal( :exec, :id => 'nested IsolatedScriptingContainer', + :arguments => [ '-jar', 'runnable.jar', 'nested.rb' ] ) end end diff --git a/maven/jruby/src/it/runnable/nested.rb b/maven/jruby/src/it/runnable/nested.rb new file mode 100644 index 00000000000..18ca7a5f18e --- /dev/null +++ b/maven/jruby/src/it/runnable/nested.rb @@ -0,0 +1,14 @@ +# TODO needs fix in jruby +#require_relative 'test' +require 'uri:classloader:/test' + +# not use Singleton scope since in this case it would use the on global runtime +other = org.jruby.embed.IsolatedScriptingContainer.new(org.jruby.embed.LocalContextScope::THREADSAFE) + +other.runScriptlet("$other = #{JRuby.runtime.object_id}") + +# TODO needs fix in jruby +#other.runScriptlet( "require_relative 'test_other'" ) +other.runScriptlet( "require 'uri:classloader:/test_other'" ) + +other.terminate diff --git a/maven/jruby/src/it/runnable/pom.xml b/maven/jruby/src/it/runnable/pom.xml index 75f70dce359..8dbcc2f7d18 100644 --- a/maven/jruby/src/it/runnable/pom.xml +++ b/maven/jruby/src/it/runnable/pom.xml @@ -9,7 +9,7 @@ true @project.version@ - 1.0.8 + 1.0.10 utf-8 pom.xml @@ -29,7 +29,7 @@ de.saumya.mojo jruby-mains - 0.2.0 + 0.3.1 @@ -92,6 +92,7 @@ true .rspec + *.rb config.ru *file *file.lock @@ -127,7 +128,7 @@ jar 1.49 - ${project.build.outputDirectory}/org/bouncycastle/bcpkix-jdk15on/1.49 + ${project.build.outputDirectory}/jars/org/bouncycastle/bcpkix-jdk15on/1.49 org.slf4j @@ -135,7 +136,7 @@ jar 1.6.4 - ${project.build.outputDirectory}/org/slf4j/slf4j-simple/1.6.4 + ${project.build.outputDirectory}/jars/org/slf4j/slf4j-simple/1.6.4 org.bouncycastle @@ -143,7 +144,7 @@ jar 1.49 - ${project.build.outputDirectory}/org/bouncycastle/bcprov-jdk15on/1.49 + ${project.build.outputDirectory}/jars/org/bouncycastle/bcprov-jdk15on/1.49 org.slf4j @@ -151,7 +152,7 @@ jar 1.6.4 - ${project.build.outputDirectory}/org/slf4j/slf4j-api/1.6.4 + ${project.build.outputDirectory}/jars/org/slf4j/slf4j-api/1.6.4 @@ -233,6 +234,20 @@ + + nested IsolatedScriptingContainer + package + + exec + + + + -jar + runnable.jar + nested.rb + + + java diff --git a/maven/jruby/src/it/runnable/spec/one_spec.rb b/maven/jruby/src/it/runnable/spec/one_spec.rb index c13eb65b385..9e7ea564828 100644 --- a/maven/jruby/src/it/runnable/spec/one_spec.rb +++ b/maven/jruby/src/it/runnable/spec/one_spec.rb @@ -4,7 +4,7 @@ it "does something" do expect(__FILE__).to eq 'uri:classloader:/spec/one_spec.rb' expect($CLASSPATH.size).to eq 4 - expect(Jars.home).to eq 'uri:classloader://' + expect(Jars.home).to eq 'uri:classloader://jars' expect(Dir.pwd).to eq 'uri:classloader://' $LOAD_PATH.each do |lp| # bundler or someone else messes up the $LOAD_PATH diff --git a/maven/jruby/src/it/runnable/test.rb b/maven/jruby/src/it/runnable/test.rb new file mode 100644 index 00000000000..fd1dae143a1 --- /dev/null +++ b/maven/jruby/src/it/runnable/test.rb @@ -0,0 +1,25 @@ +require 'minitest/autorun' + +describe 'ENV' do + + it 'has current directory inside classloader' do + Dir.pwd.must_equal 'uri:classloader://' + end + + it 'has first entry LOAD_PATH' do + $LOAD_PATH.first.must_equal 'uri:classloader://' + end + + it 'has GEM_HOME set' do + ENV['GEM_HOME'].must_equal 'uri:classloader://META-INF/jruby.home/lib/ruby/gems/shared' + end + + it 'has GEM_PATH set' do + ENV['GEM_PATH'].must_equal 'uri:classloader://' + end + + it 'has JARS_HOME set' do + ENV['JARS_HOME'].must_equal 'uri:classloader://jars' + end + +end diff --git a/maven/jruby/src/it/runnable/test_other.rb b/maven/jruby/src/it/runnable/test_other.rb new file mode 100644 index 00000000000..e1af6c02988 --- /dev/null +++ b/maven/jruby/src/it/runnable/test_other.rb @@ -0,0 +1,29 @@ +require 'minitest/autorun' + +describe 'ENV' do + + it 'has $other' do + $other.wont_equal JRuby.runtime.object_id + end + + it 'has first entry LOAD_PATH' do + $LOAD_PATH.first.must_equal 'uri:classloader:/' + end + + it 'has current directory inside classloader' do + Dir.pwd.must_equal 'uri:classloader:/' + end + + it 'has GEM_HOME set' do + ENV['GEM_HOME'].must_equal 'uri:classloader:/' + end + + it 'has GEM_PATH set' do + ENV['GEM_PATH'].must_equal 'uri:classloader:/' + end + + it 'has JARS_HOME set' do + ENV['JARS_HOME'].must_equal 'uri:classloader:/jars' + end + +end diff --git a/maven/jruby/src/it/runnable/verify.bsh b/maven/jruby/src/it/runnable/verify.bsh index 988b42f9a2b..c217b1316e1 100644 --- a/maven/jruby/src/it/runnable/verify.bsh +++ b/maven/jruby/src/it/runnable/verify.bsh @@ -10,4 +10,16 @@ expected = "uri:classloader:/Rakefile []"; if ( !log.contains( expected ) ) throw new RuntimeException( "log file does not contain '" + expected + "'" ); expected = "uri:classloader:/Rakefile [\"spec\"]"; -if ( !log.contains( expected ) ) throw new RuntimeException( "log file does not contain '" + expected + "'" ); \ No newline at end of file +if ( !log.contains( expected ) ) throw new RuntimeException( "log file does not contain '" + expected + "'" ); + +expected = "5 tests, 5 assertions, 0 failures, 0 errors, 0 skips"; +if ( !log.contains( expected ) ) +{ + throw new RuntimeException( "log file does not contain '" + expected + "'" ); +} + +expected = "6 tests, 6 assertions, 0 failures, 0 errors, 0 skips"; +if ( !log.contains( expected ) ) +{ + throw new RuntimeException( "log file does not contain '" + expected + "'" ); +} \ No newline at end of file From bef5abfc2b4a5de5132f789109efcbdfe27f5e6d Mon Sep 17 00:00:00 2001 From: Christian Meier Date: Fri, 24 Jul 2015 10:34:00 +0200 Subject: [PATCH 27/40] get classloader root url right without an trailing slash OSGi url protocols do not like double // after the "host" part of the url. --- .../main/java/org/jruby/embed/IsolatedScriptingContainer.java | 2 +- core/src/main/java/org/jruby/javasupport/JavaEmbedUtils.java | 1 - core/src/main/java/org/jruby/util/UriLikePathHelper.java | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/jruby/embed/IsolatedScriptingContainer.java b/core/src/main/java/org/jruby/embed/IsolatedScriptingContainer.java index 068ff3958b0..053e5f13230 100644 --- a/core/src/main/java/org/jruby/embed/IsolatedScriptingContainer.java +++ b/core/src/main/java/org/jruby/embed/IsolatedScriptingContainer.java @@ -124,7 +124,7 @@ protected void addLoadPath(String uri) { } public void addGemPath( ClassLoader cl ) { - addGemPath(new UriLikePathHelper(cl).getUriLikePath()); + addGemPath(new UriLikePathHelper(cl).getUriLikePath("/specifications" + JRUBYDIR)); } public void addGemPath( ClassLoader cl, String ref ) { diff --git a/core/src/main/java/org/jruby/javasupport/JavaEmbedUtils.java b/core/src/main/java/org/jruby/javasupport/JavaEmbedUtils.java index 1d8a9246ef2..2d538cf228b 100644 --- a/core/src/main/java/org/jruby/javasupport/JavaEmbedUtils.java +++ b/core/src/main/java/org/jruby/javasupport/JavaEmbedUtils.java @@ -28,7 +28,6 @@ * the terms of any one of the EPL, the GPL or the LGPL. ***** END LICENSE BLOCK *****/ -import java.io.IOException; import java.io.InputStream; import java.util.List; diff --git a/core/src/main/java/org/jruby/util/UriLikePathHelper.java b/core/src/main/java/org/jruby/util/UriLikePathHelper.java index 5fde4c67878..a66276f20f4 100644 --- a/core/src/main/java/org/jruby/util/UriLikePathHelper.java +++ b/core/src/main/java/org/jruby/util/UriLikePathHelper.java @@ -45,7 +45,7 @@ public URL getResource(String ref) { } public String getUriLikePath() { - return createUri(".jrubydir"); + return createUri("/.jrubydir"); } public String getUriLikePath(String ref) { From 917d0e7ddad7ddbc792a274ebed3b933b3b6453f Mon Sep 17 00:00:00 2001 From: Christian Meier Date: Fri, 24 Jul 2015 12:47:54 +0200 Subject: [PATCH 28/40] fixes commandline execution of internal java -cp ... org.jruby.Main command it is a regression from 1.7.21 since gems with native extensions (thrift-0.9.2.0) can not be installed with jruby-complete anymore. this patch treats the command as series of arguments in case of the internal "jruby" command gets executed. Sponsored by Lookout Inc. --- .../java/org/jruby/util/ShellLauncher.java | 5 + maven/jruby-complete/src/it/integrity/pom.xml | 257 ++++++++++-------- 2 files changed, 144 insertions(+), 118 deletions(-) diff --git a/core/src/main/java/org/jruby/util/ShellLauncher.java b/core/src/main/java/org/jruby/util/ShellLauncher.java index 2041c203998..ab28644bee3 100644 --- a/core/src/main/java/org/jruby/util/ShellLauncher.java +++ b/core/src/main/java/org/jruby/util/ShellLauncher.java @@ -1178,6 +1178,11 @@ public void verifyExecutableForShell() { execArgs[0] = shell; execArgs[1] = shell.endsWith("sh") ? "-c" : "/c"; + // HACK to get the builtin command right + if (cmdline.startsWith("\"java -cp") && cmdline.contains("org.jruby.Main\"")) { + cmdline = cmdline.replace("\"java -cp", "java -cp") + .replace("org.jruby.Main\"", "org.jruby.Main"); + } if (Platform.IS_WINDOWS) { // that's how MRI does it too execArgs[2] = "\"" + cmdline + "\""; diff --git a/maven/jruby-complete/src/it/integrity/pom.xml b/maven/jruby-complete/src/it/integrity/pom.xml index ddf5a8850a0..6a167010fa1 100644 --- a/maven/jruby-complete/src/it/integrity/pom.xml +++ b/maven/jruby-complete/src/it/integrity/pom.xml @@ -19,145 +19,166 @@ org.codehaus.mojo exec-maven-plugin 1.2 - - java - - ${basedir} - ${basedir} - ${basedir} - ${basedir} - - - - - gem list - test - + + java + + ${basedir}/doe2notex1st2 + ${basedir} + ${basedir} + + + + + gem list + test + exec - - - -classpath - - org.jruby.Main - -S - gem - list - - - - - rake executable - test - + + + -classpath + + org.jruby.Main + -S + gem + list + + + + + rake executable + test + exec - - - -classpath - - org.jruby.Main - -S - rake - --version - - - - - rdoc executable - test - + + + -classpath + + org.jruby.Main + -S + rake + --version + + + + + rdoc executable + test + exec - - - -classpath - - org.jruby.Main - -S - rdoc - --version - - - - - count installed gems - test - + + + -classpath + + org.jruby.Main + -S + rdoc + --version + + + + + count installed gems + test + exec - - - -classpath - - org.jruby.Main - -e - - require 'stringio' - require 'rubygems/commands/list_command' - require 'rubygems/user_interaction' - s = StringIO.new - l = Gem::Commands::ListCommand.new - l.ui= Gem::StreamUI.new( STDIN, s, STDERR, true ) - l.execute - c = s.string.split( /\n/ ).count - puts 'gems count ' + c.to_s - - - - - - load default gems - test - + + + -classpath + + org.jruby.Main + -e + + require 'stringio' + require 'rubygems/commands/list_command' + require 'rubygems/user_interaction' + s = StringIO.new + l = Gem::Commands::ListCommand.new + l.ui= Gem::StreamUI.new( STDIN, s, STDERR, true ) + l.execute + c = s.string.split( /\n/ ).count + puts 'gems count ' + c.to_s + + + + + + load default gems + test + exec - java + java - -classpath - - - org.jruby.Main - -e - - - require 'jar-dependencies' - require 'openssl' - puts Gem.loaded_specs.keys.sort.join(',') - + -classpath + + + org.jruby.Main + -e + + + require 'jar-dependencies' + require 'openssl' + puts Gem.loaded_specs.keys.sort.join(',') + - - - ensure there is no org.objectweb.asm.ClassWriter - test - + + + ensure there is no org.objectweb.asm.ClassWriter + test + exec - java + java - -classpath - - - org.jruby.Main - -e - - - begin - import_java "org.objectweb.asm.ClassWriter" - raise "error there is org.objectweb.asm.ClassWriter on the classpath" - rescue NameError => e - puts "there is NO org.objectweb.asm.ClassWriter on the classpath" + -classpath + + + org.jruby.Main + -e + + + begin + import_java "org.objectweb.asm.ClassWriter" + raise "error there is org.objectweb.asm.ClassWriter on the classpath" + rescue NameError => e + puts "there is NO org.objectweb.asm.ClassWriter on the classpath" end - + + + + + + gem install thrift -v0.9.2.0 - GH-3141 + test + + exec + + + java + + -classpath + + + org.jruby.Main + -S + gem + install + thrift + -v0.9.2.0 - - + + From 0832a7c997cd47a4a558282efc6cc14313dfa555 Mon Sep 17 00:00:00 2001 From: Christian Meier Date: Fri, 24 Jul 2015 14:37:20 +0200 Subject: [PATCH 29/40] detect uri-like paths better for File.expand_path whether second argument of File.expand_path('foo', Pathname.new('uri:classloader:/')) is ruby string or pathname, the result should be the same. fixes #3150 Sponsored by Lookout Inc. --- core/src/main/java/org/jruby/RubyFile.java | 5 +++-- test/test_file.rb | 6 ++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/jruby/RubyFile.java b/core/src/main/java/org/jruby/RubyFile.java index 8edbc449939..1bf9c5a295a 100644 --- a/core/src/main/java/org/jruby/RubyFile.java +++ b/core/src/main/java/org/jruby/RubyFile.java @@ -1654,8 +1654,9 @@ private static IRubyObject expandPathInternal(ThreadContext context, IRubyObject // argument is relative. if (args.length == 2 && !args[1].isNil()) { // TODO maybe combine this with get_path method - if ((args[1] instanceof RubyString) && args[1].asJavaString().startsWith("uri:")) { - cwd = args[1].asJavaString(); + String path = args[1].toString(); + if (path.startsWith("uri:")) { + cwd = path; } else { cwd = StringSupport.checkEmbeddedNulls(runtime, get_path(context, args[1])).getUnicodeValue(); diff --git a/test/test_file.rb b/test/test_file.rb index 1866a10009b..84eb77d9239 100644 --- a/test/test_file.rb +++ b/test/test_file.rb @@ -325,6 +325,12 @@ def test_expand_path_with_file_url_relative_path assert_equal "file:#{Dir.pwd}/foo/bar", File.expand_path("file:foo/bar") end + # GH-3150 + def test_expand_path_with_pathname_and_uri_path + jruby_specific_test + assert_equal 'uri:classloader://foo', File.expand_path('foo', Pathname.new('uri:classloader:/')) + end + # JRUBY-5219 def test_expand_path_looks_like_url jruby_specific_test From 21a9cbe3b8d53a929474d9c64c4541afc71c9738 Mon Sep 17 00:00:00 2001 From: Christian Meier Date: Fri, 24 Jul 2015 15:45:40 +0200 Subject: [PATCH 30/40] fixes expand_path on relative reference and inside uri:classloader: as CWD using JRubyFile instead of regular File helps to keep the uri-like paths intact. fixes #3176 Sponsored by Lookout Inc. --- core/src/main/java/org/jruby/RubyFile.java | 6 ++++-- core/src/main/ruby/jruby/kernel19/kernel.rb | 13 +------------ test/test_file.rb | 9 +++++++++ 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/org/jruby/RubyFile.java b/core/src/main/java/org/jruby/RubyFile.java index 1bf9c5a295a..28018ee1e7b 100644 --- a/core/src/main/java/org/jruby/RubyFile.java +++ b/core/src/main/java/org/jruby/RubyFile.java @@ -1664,7 +1664,8 @@ private static IRubyObject expandPathInternal(ThreadContext context, IRubyObject if (expandUser) { cwd = expandUserPath(context, cwd, true); } - + + // TODO try to treat all uri-like paths alike String[] cwdURIParts = splitURI(cwd); if (uriParts == null && cwdURIParts != null) { uriParts = cwdURIParts; @@ -1681,7 +1682,8 @@ private static IRubyObject expandPathInternal(ThreadContext context, IRubyObject // If the path isn't absolute, then prepend the current working // directory to the path. if (!startsWithSlashNotOnWindows && !startsWithDriveLetterOnWindows(cwd)) { - cwd = new File(runtime.getCurrentDirectory(), cwd).getAbsolutePath(); + if ("".equals(cwd)) cwd = "."; + cwd = JRubyFile.create(runtime.getCurrentDirectory(), cwd).getAbsolutePath(); } } } else { diff --git a/core/src/main/ruby/jruby/kernel19/kernel.rb b/core/src/main/ruby/jruby/kernel19/kernel.rb index ffb1b1fb5b9..2019db2eacd 100644 --- a/core/src/main/ruby/jruby/kernel19/kernel.rb +++ b/core/src/main/ruby/jruby/kernel19/kernel.rb @@ -8,18 +8,7 @@ def require_relative(relative_arg) file = $` # just the filename raise LoadError, "cannot infer basepath" if /\A\((.*)\)/ =~ file # eval etc. - # FIXME: Classpath-path can be removed if we make expand_path+dirname - # know about classpath: paths. - if file =~ /^classpath:(.*)/ - dir = File.dirname($1) - dir = dir == '.' ? "" : dir + "/" - absolute_feature = "classpath:#{dir}#{relative_arg}" - elsif file =~ /^uri:(.*)/ - dir = File.dirname($1) - absolute_feature = "uri:#{dir}/#{relative_arg}" - else - absolute_feature = File.expand_path(relative_arg, File.dirname(file)) - end + absolute_feature = File.expand_path(relative_arg, File.dirname(file)) require absolute_feature end diff --git a/test/test_file.rb b/test/test_file.rb index 84eb77d9239..8a6b5f7da9e 100644 --- a/test/test_file.rb +++ b/test/test_file.rb @@ -331,6 +331,15 @@ def test_expand_path_with_pathname_and_uri_path assert_equal 'uri:classloader://foo', File.expand_path('foo', Pathname.new('uri:classloader:/')) end + # GH-3176 + def test_expand_path_with_relative_reference_and_inside_uri_classloader + jruby_specific_test + Dir.chdir( 'uri:classloader:/') do + assert_equal 'uri:classloader://something/foo', File.expand_path('foo', 'something') + assert_equal 'uri:classloader://foo', File.expand_path('foo', '.') + end + end + # JRUBY-5219 def test_expand_path_looks_like_url jruby_specific_test From f77aea2b6dc2ffad53ff0f49d11f39de3a6f8508 Mon Sep 17 00:00:00 2001 From: Joel Segerlind Date: Tue, 21 Jul 2015 23:32:30 +0200 Subject: [PATCH 31/40] Define packages for classes in nested JARs Packages are defined in the private `defineClass(String name, Resource res)` of `URLClassLoader`, which is only called from `findClass(final String name)`. Thus, whenever `findClass()` in `URLClassLoader` throws `ClassNotFoundException` (e.g. for JARs within JARs), the package doesn't get defined, even if `findClass(String className)` in `JRubyClassLoader` is able to find and define the sought class. I've used http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/687fd7c7986d/src/share/classes/java/net/URLClassLoader.java for reference. --- .../java/org/jruby/util/JRubyClassLoader.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/core/src/main/java/org/jruby/util/JRubyClassLoader.java b/core/src/main/java/org/jruby/util/JRubyClassLoader.java index 93506cfa031..a01b61ac843 100644 --- a/core/src/main/java/org/jruby/util/JRubyClassLoader.java +++ b/core/src/main/java/org/jruby/util/JRubyClassLoader.java @@ -210,6 +210,11 @@ protected Class findClass(String className) throws ClassNotFoundException { } byte[] data = output.toByteArray(); + int dotIndex = className.lastIndexOf("."); + if (dotIndex != -1) { + String packageName = className.substring(0, dotIndex); + definePackageInternal(packageName); + } return defineClass(className, data, 0, data.length); } finally { close(input); @@ -329,4 +334,16 @@ private static void close(Closeable resource) { } } } + + private void definePackageInternal(String pkgname) { + if (getPackage(pkgname) == null) { + try { + definePackage(pkgname, null, null, null, null, null, null, null); + } catch (IllegalArgumentException iae) { + if (getPackage(pkgname) == null) { + throw new AssertionError("Cannot find package " + pkgname); + } + } + } + } } From c0a9e8e9124939341bfba830306e6bbad917e427 Mon Sep 17 00:00:00 2001 From: Joel Segerlind Date: Fri, 24 Jul 2015 22:54:20 +0200 Subject: [PATCH 32/40] Add test for packages of classes in nested JARs The example would fail unless the package has been defined, as JavaClass#package would return nil, which obviously doesn't define the method #name. --- test/test_load.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/test_load.rb b/test/test_load.rb index 35a1a2e0851..7270d095c2a 100644 --- a/test/test_load.rb +++ b/test/test_load.rb @@ -103,6 +103,15 @@ def test_require_nested_jar_enables_class_loading_from_that_jar } end + def test_require_nested_jar_defines_packages_for_classes_in_that_jar + assert_equal "test", run_in_sub_runtime(%{ + require 'test/jar_with_nested_classes_jar' + require 'jar_with_classes' + java_import "test.HelloThere" + HelloThere.java_class.package.name + }) + end + def call_extern_load_foo_bar(classpath = nil) cmd = "" cmd += "env CLASSPATH=#{classpath}" # classpath=nil, becomes empty CLASSPATH From 2445a8006e835d81bd9ceb184a507e0f3949cae1 Mon Sep 17 00:00:00 2001 From: kares Date: Sat, 25 Jul 2015 11:04:05 +0200 Subject: [PATCH 33/40] JavaMethod's (redundant) arityValue field is only used in ReflectedJavaMethod --- .../internal/runtime/methods/JavaMethod.java | 12 +++--- .../runtime/methods/ReflectedJavaMethod.java | 37 ++++++++++--------- 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/core/src/main/java/org/jruby/internal/runtime/methods/JavaMethod.java b/core/src/main/java/org/jruby/internal/runtime/methods/JavaMethod.java index f8dc5dfdc53..7999142a1a4 100644 --- a/core/src/main/java/org/jruby/internal/runtime/methods/JavaMethod.java +++ b/core/src/main/java/org/jruby/internal/runtime/methods/JavaMethod.java @@ -37,7 +37,7 @@ /** */ public abstract class JavaMethod extends DynamicMethod implements Cloneable, MethodArgs2 { - protected int arityValue; + protected Arity arity = Arity.OPTIONAL; private String javaName; private boolean isSingleton; @@ -45,9 +45,9 @@ public abstract class JavaMethod extends DynamicMethod implements Cloneable, Met private String parameterDesc; private String[] parameterList; - private static String[] ONE_REQ = new String[] { "q" }; - private static String[] TWO_REQ = new String[] { "q", "q" }; - private static String[] THREE_REQ = new String[] { "q", "q", "q" }; + private static final String[] ONE_REQ = new String[] { "q" }; + private static final String[] TWO_REQ = new String[] { "q", "q" }; + private static final String[] THREE_REQ = new String[] { "q", "q", "q" }; public static final Class[][] METHODS = { {JavaMethodZero.class, JavaMethodZeroOrOne.class, JavaMethodZeroOrOneOrTwo.class, JavaMethodZeroOrOneOrTwoOrThree.class}, @@ -94,8 +94,7 @@ protected JavaMethod() {} public void init(RubyModule implementationClass, Arity arity, Visibility visibility, StaticScope staticScope, CallConfiguration callConfig) { this.staticScope = staticScope; - this.arity = arity; - this.arityValue = arity.getValue(); + setArity(arity); super.init(implementationClass, visibility, callConfig); } @@ -190,7 +189,6 @@ protected final void returnTraceCompiled(ThreadContext context, boolean enabled, public void setArity(Arity arity) { this.arity = arity; - this.arityValue = arity.getValue(); } @Override diff --git a/core/src/main/java/org/jruby/internal/runtime/methods/ReflectedJavaMethod.java b/core/src/main/java/org/jruby/internal/runtime/methods/ReflectedJavaMethod.java index 29b55bcb5ae..d96e5f327e4 100644 --- a/core/src/main/java/org/jruby/internal/runtime/methods/ReflectedJavaMethod.java +++ b/core/src/main/java/org/jruby/internal/runtime/methods/ReflectedJavaMethod.java @@ -12,7 +12,7 @@ * rights and limitations under the License. * * Copyright (c) 2007 Peter Brant - * + * * Alternatively, the contents of this file may be used under the terms of * either of the GNU General Public License Version 2 or later (the "GPL"), * or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), @@ -44,38 +44,41 @@ public class ReflectedJavaMethod extends JavaMethod { private final Method method; - + private final boolean needsBlock; private final boolean isStatic; private final int required; private final int optional; private final boolean rest; private final int max; - + private final boolean argsAsIs; private final boolean needsThreadContext; + final int arityValue; + public ReflectedJavaMethod( RubyModule implementationClass, Method method, JRubyMethod annotation) { super(implementationClass, annotation.visibility()); - + this.method = method; - + Class[] params = method.getParameterTypes(); this.needsBlock = params.length > 0 && params[params.length - 1] == Block.class; this.isStatic = Modifier.isStatic(method.getModifiers()); - + Arity arity = Arity.fromAnnotation(annotation, params, this.isStatic); setArity(arity); + this.arityValue = arity.getValue(); - this.required = arity.getValue() >= 0 ? arity.getValue() : Math.abs(arity.getValue())-1; + this.required = arityValue >= 0 ? arityValue : ( - arityValue ) - 1; this.optional = annotation.optional(); this.rest = annotation.rest(); - + this.needsThreadContext = params.length > 0 && params[0] == ThreadContext.class; this.argsAsIs = ! isStatic && optional == 0 && !rest && !needsBlock && !needsThreadContext; - + if (rest) { max = -1; } else { @@ -88,31 +91,31 @@ public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule claz IRubyObject[] args, Block block) { Ruby runtime = context.runtime; Arity.checkArgumentCount(runtime, name, args, required, max); - + callConfig.pre(context, self, getImplementationClass(), name, block, null); - + try { if (! isStatic && ! method.getDeclaringClass().isAssignableFrom(self.getClass())) { throw new ClassCastException( self.getClass().getName() + " cannot be converted to " + method.getDeclaringClass().getName()); } - + if (argsAsIs) { boolean isTrace = runtime.hasEventHooks(); try { if (isTrace) { runtime.callEventHooks(context, RubyEvent.C_CALL, context.getFile(), context.getLine(), name, getImplementationClass()); - } + } return (IRubyObject)method.invoke(self, (Object[])args); } finally { if (isTrace) { runtime.callEventHooks(context, RubyEvent.C_RETURN, context.getFile(), context.getLine(), name, getImplementationClass()); } - } + } } else { int argsLength = calcArgsLength(); - + Object[] params = new Object[argsLength]; int offset = 0; if (needsThreadContext) { @@ -135,7 +138,7 @@ public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule claz if (needsBlock) { params[offset++] = block; } - + boolean isTrace = runtime.hasEventHooks(); try { if (isTrace) { @@ -175,7 +178,7 @@ public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule claz private int calcArgsLength() { int argsLength = 0; - + if (needsThreadContext) { argsLength++; } From 48e8096980f86a5121678bda34e42412e25125e0 Mon Sep 17 00:00:00 2001 From: kares Date: Sat, 25 Jul 2015 15:10:28 +0200 Subject: [PATCH 34/40] correct javadoc and generix-ize the guts of WeakValued map-like collections --- .../collections/WeakValuedIdentityMap.java | 7 ++-- .../jruby/util/collections/WeakValuedMap.java | 38 +++++++++---------- 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/core/src/main/java/org/jruby/util/collections/WeakValuedIdentityMap.java b/core/src/main/java/org/jruby/util/collections/WeakValuedIdentityMap.java index 344f057574e..e6fd92d9e13 100644 --- a/core/src/main/java/org/jruby/util/collections/WeakValuedIdentityMap.java +++ b/core/src/main/java/org/jruby/util/collections/WeakValuedIdentityMap.java @@ -30,17 +30,16 @@ ***** END LICENSE BLOCK *****/ package org.jruby.util.collections; -import java.lang.ref.ReferenceQueue; -import java.lang.ref.WeakReference; import java.util.Collections; import java.util.IdentityHashMap; import java.util.Map; /** - * A Map that holds its values weakly and uses object identity for keys. + * Map-like that holds its values weakly and uses object identity for keys. */ public class WeakValuedIdentityMap extends WeakValuedMap { + @Override protected Map> newMap() { - return Collections.synchronizedMap(new IdentityHashMap()); + return Collections.synchronizedMap( new IdentityHashMap>() ); } } diff --git a/core/src/main/java/org/jruby/util/collections/WeakValuedMap.java b/core/src/main/java/org/jruby/util/collections/WeakValuedMap.java index 5bd06ab466b..1f62c84aef1 100644 --- a/core/src/main/java/org/jruby/util/collections/WeakValuedMap.java +++ b/core/src/main/java/org/jruby/util/collections/WeakValuedMap.java @@ -32,52 +32,50 @@ import java.lang.ref.ReferenceQueue; import java.lang.ref.WeakReference; -import java.util.IdentityHashMap; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; /** - * A Map that holds its values weakly and uses object identity for keys. + * Map-like that holds its values weakly (backed by a concurrent hash map). */ public class WeakValuedMap { - private final ReferenceQueue deadReferences = new ReferenceQueue(); - private final Map> references = newMap(); - public void put(Key key, Value value) { + private final Map> map = newMap(); + @SuppressWarnings("unchecked") + private final ReferenceQueue deadRefs = new ReferenceQueue(); + + public final void put(Key key, Value value) { cleanReferences(); - references.put(key, new KeyedReference(value, key, deadReferences)); + map.put(key, new KeyedReference(value, key, deadRefs)); } - public Value get(Key key) { + public final Value get(Key key) { cleanReferences(); - KeyedReference reference = references.get(key); - if (reference == null) { - return null; - } + KeyedReference reference = map.get(key); + if (reference == null) return null; return reference.get(); } protected Map> newMap() { - return new ConcurrentHashMap(); + return new ConcurrentHashMap>(); } protected static class KeyedReference extends WeakReference { - private final Key key; - public KeyedReference(Value object, Key key, ReferenceQueue queue) { + protected final Key key; + + public KeyedReference(Value object, Key key, ReferenceQueue queue) { super(object, queue); this.key = key; } - public Key key() { - return key; - } } + @SuppressWarnings("unchecked") private void cleanReferences() { - KeyedReference ref; - while ((ref = (KeyedReference) deadReferences.poll()) != null) { - references.remove((ref.key())); + KeyedReference ref; + while ( ( ref = (KeyedReference) deadRefs.poll() ) != null ) { + map.remove( ref.key ); } } } From 59c5e4bdf51fbd3adff67467f1c57451e1abecb7 Mon Sep 17 00:00:00 2001 From: kares Date: Sat, 25 Jul 2015 15:37:49 +0200 Subject: [PATCH 35/40] review the WeakHashSet impl + add missing (useful) methods: equals, toString, ... --- .../jruby/util/collections/WeakHashSet.java | 105 +++++++++++++----- 1 file changed, 77 insertions(+), 28 deletions(-) diff --git a/core/src/main/java/org/jruby/util/collections/WeakHashSet.java b/core/src/main/java/org/jruby/util/collections/WeakHashSet.java index c87ae6ae640..da22478ce6d 100644 --- a/core/src/main/java/org/jruby/util/collections/WeakHashSet.java +++ b/core/src/main/java/org/jruby/util/collections/WeakHashSet.java @@ -25,28 +25,40 @@ ***** END LICENSE BLOCK *****/ package org.jruby.util.collections; -import java.util.*; +import java.util.Collection; +import java.util.Iterator; +import java.util.Set; +import java.util.WeakHashMap; /** * A simple set that uses weak references to ensure that its elements can be garbage collected. - * See WeakHashMap. + * + * @see java.util.WeakHashMap + * @see java.util.HashSet * * @author Robert Egglestone */ -public class WeakHashSet implements Set { - private WeakHashMap map; +public class WeakHashSet implements Set, Cloneable { + + // Dummy value to associate with an Object in the backing Map + private static final Object PRESENT = Boolean.TRUE; + + private final WeakHashMap map; public WeakHashSet() { - map = new WeakHashMap(); + map = new WeakHashMap(); } public WeakHashSet(int size) { - map = new WeakHashMap(size); + map = new WeakHashMap(size); } public boolean add(T o) { - Object previousValue = map.put(o, null); - return previousValue == null; + return map.put(o, PRESENT) == null; + } + + public boolean remove(Object o) { + return map.remove(o) == PRESENT; } public Iterator iterator() { @@ -61,47 +73,84 @@ public boolean isEmpty() { return map.isEmpty(); } + public void clear() { + map.clear(); + } + public boolean contains(Object o) { return map.containsKey(o); } - public boolean remove(Object o) { - boolean contains = contains(o); - map.remove(o); - return contains; + public boolean containsAll(Collection coll) { + return map.keySet().containsAll(coll); } - public boolean removeAll(Collection collection) { - return map.keySet().removeAll(collection); + public boolean removeAll(Collection coll) { + return map.keySet().removeAll(coll); } - public boolean retainAll(Collection collection) { - return map.keySet().retainAll(collection); + public boolean retainAll(Collection coll) { + return map.keySet().retainAll(coll); } - public void clear() { - map.clear(); + public boolean addAll(Collection coll) { + boolean added = false; + for (T i: coll) { + add(i); + added = true; + } + return added; } public Object[] toArray() { return map.keySet().toArray(); } - public boolean containsAll(Collection arg0) { - return map.keySet().containsAll(arg0); + public T[] toArray(final T[] arr) { + return map.keySet().toArray(arr); } - public boolean addAll(Collection arg0) { - boolean added = false; - for (T i: arg0) { - add(i); - added = true; + @Override + public boolean equals(final Object o) { + if ( o == this ) return true; + if ( o instanceof Set ) { + final Set that = (Set) o; + if ( that.size() != this.size() ) return false; + try { + return containsAll(that); + } + catch (ClassCastException ignore) { /* return false; */ } } - return added; + return false; + } + + @Override + public int hashCode() { + return 11 * map.hashCode(); } - public T[] toArray(T[] arg0) { - return map.keySet().toArray(arg0); + @Override + public WeakHashSet clone() { + //WeakHashSet newSet = (WeakHashSet) super.clone(); + //newSet.map = (WeakHashMap) map.clone(); + WeakHashSet newSet = new WeakHashSet(map.size()); + newSet.map.putAll(this.map); + return newSet; + } + + @Override + public String toString() { + Iterator it = iterator(); + if ( ! it.hasNext() ) return "[]"; + + StringBuilder sb = new StringBuilder(); + sb.append('['); + while (true) { + final T e = it.next(); + sb.append(e == this ? "(this Collection)" : e); + if ( ! it.hasNext() ) return sb.append(']').toString(); + sb.append(',').append(' '); + } } } From b69e4f8c362c4d2590291c6d3aaeb584f477f5a8 Mon Sep 17 00:00:00 2001 From: kares Date: Sat, 25 Jul 2015 16:39:21 +0200 Subject: [PATCH 36/40] depreacate JRubyClassLoader's getJDBCDriverUnloader (its un-used - but still should not be part of public API) --- core/src/main/java/org/jruby/util/JRubyClassLoader.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/jruby/util/JRubyClassLoader.java b/core/src/main/java/org/jruby/util/JRubyClassLoader.java index a01b61ac843..515b21499fc 100644 --- a/core/src/main/java/org/jruby/util/JRubyClassLoader.java +++ b/core/src/main/java/org/jruby/util/JRubyClassLoader.java @@ -58,12 +58,13 @@ public class JRubyClassLoader extends URLClassLoader implements ClassDefiningCla private static final Logger LOG = LoggerFactory.getLogger("JRubyClassLoader"); private final static ProtectionDomain DEFAULT_DOMAIN; - + static { ProtectionDomain defaultDomain = null; try { defaultDomain = JRubyClassLoader.class.getProtectionDomain(); - } catch (SecurityException se) { + } + catch (SecurityException se) { // just use null since we can't acquire protection domain } DEFAULT_DOMAIN = defaultDomain; @@ -149,6 +150,7 @@ public void tearDown(boolean debug) { } } + @Deprecated public synchronized Runnable getJDBCDriverUnloader() { if (unloader == null) { try { From bcbef59e6bf9dc4f39fbea5b66f9f732efad142a Mon Sep 17 00:00:00 2001 From: kares Date: Sat, 25 Jul 2015 16:57:15 +0200 Subject: [PATCH 37/40] only re-wrap non-runtime exceptions in getJDBCDriverUnloader --- .../main/java/org/jruby/util/JRubyClassLoader.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/jruby/util/JRubyClassLoader.java b/core/src/main/java/org/jruby/util/JRubyClassLoader.java index 515b21499fc..ab5965267f0 100644 --- a/core/src/main/java/org/jruby/util/JRubyClassLoader.java +++ b/core/src/main/java/org/jruby/util/JRubyClassLoader.java @@ -143,10 +143,9 @@ public void tearDown(boolean debug) { // A hack to allow unloading all JDBC Drivers loaded by this classloader. // See http://bugs.jruby.org/4226 getJDBCDriverUnloader().run(); - } catch (Exception e) { - if (debug) { - LOG.debug(e); - } + } + catch (Exception e) { + if (debug) LOG.debug(e); } } @@ -164,9 +163,9 @@ public synchronized Runnable getJDBCDriverUnloader() { Class unloaderClass = defineClass("org.jruby.util.JDBCDriverUnloader", baos.toByteArray()); unloader = (Runnable) unloaderClass.newInstance(); - } catch (Exception e) { - throw new RuntimeException(e); } + catch (RuntimeException e) { throw e; } + catch (Exception e) { throw new RuntimeException(e); } } return unloader; } From 2b280363c14b82cca82132f47603e2235ad55525 Mon Sep 17 00:00:00 2001 From: kares Date: Sat, 25 Jul 2015 19:28:47 +0200 Subject: [PATCH 38/40] close (URLClassLoader's) resources on JRubyClassLoader#tearDown on Java 7+ --- .../java/org/jruby/util/JRubyClassLoader.java | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/jruby/util/JRubyClassLoader.java b/core/src/main/java/org/jruby/util/JRubyClassLoader.java index ab5965267f0..947b454b6bf 100644 --- a/core/src/main/java/org/jruby/util/JRubyClassLoader.java +++ b/core/src/main/java/org/jruby/util/JRubyClassLoader.java @@ -147,6 +147,17 @@ public void tearDown(boolean debug) { catch (Exception e) { if (debug) LOG.debug(e); } + // if we're on Java 7+ call URLClassLoader#close : + try { + URLClassLoader.class.getMethod("close").invoke(this); + } + catch (NoSuchMethodException ex) { /* noop on Java 6 */ } + catch (IllegalAccessException ex) { + LOG.info("unexpected illegal access: ", ex); + } + catch (Exception ex) { + LOG.debug(ex); + } } @Deprecated @@ -331,18 +342,19 @@ private static void close(Closeable resource) { if (resource != null) { try { resource.close(); - } catch (IOException ignore) { } + catch (IOException ignore) { /* ignored */ } } } - private void definePackageInternal(String pkgname) { - if (getPackage(pkgname) == null) { + private void definePackageInternal(final String name) { + if (getPackage(name) == null) { try { - definePackage(pkgname, null, null, null, null, null, null, null); - } catch (IllegalArgumentException iae) { - if (getPackage(pkgname) == null) { - throw new AssertionError("Cannot find package " + pkgname); + definePackage(name, null, null, null, null, null, null, null); + } + catch (IllegalArgumentException iae) { + if (getPackage(name) == null) { + throw new AssertionError("Cannot find package " + name); } } } From efa6ce00488717cab3141ea4598e7d1c7a8929fc Mon Sep 17 00:00:00 2001 From: kares Date: Sat, 25 Jul 2015 20:43:59 +0200 Subject: [PATCH 39/40] un-used imports --- core/src/main/java/org/jruby/RubyInstanceConfig.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/core/src/main/java/org/jruby/RubyInstanceConfig.java b/core/src/main/java/org/jruby/RubyInstanceConfig.java index 2cb5e05c64b..72213f590b6 100644 --- a/core/src/main/java/org/jruby/RubyInstanceConfig.java +++ b/core/src/main/java/org/jruby/RubyInstanceConfig.java @@ -58,9 +58,7 @@ import java.io.BufferedReader; import java.io.ByteArrayInputStream; import java.io.File; -import java.io.FileInputStream; import java.io.FileNotFoundException; -import java.io.FileReader; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; @@ -75,8 +73,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.jar.JarEntry; -import java.util.jar.JarFile; import java.util.regex.Pattern; /** From 83c47ba357b62d15225a50845c9f1745d3940b4a Mon Sep 17 00:00:00 2001 From: kares Date: Sat, 25 Jul 2015 20:45:02 +0200 Subject: [PATCH 40/40] keep the provider in a local while terminating + missing @Override --- .../main/java/org/jruby/embed/ScriptingContainer.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/jruby/embed/ScriptingContainer.java b/core/src/main/java/org/jruby/embed/ScriptingContainer.java index 106ff6ddfb0..83c75e68423 100644 --- a/core/src/main/java/org/jruby/embed/ScriptingContainer.java +++ b/core/src/main/java/org/jruby/embed/ScriptingContainer.java @@ -29,6 +29,7 @@ */ package org.jruby.embed; +import java.io.Closeable; import java.io.InputStream; import java.io.PrintStream; import java.io.Reader; @@ -1896,8 +1897,11 @@ public PrintStream getErr() { * @since JRuby 1.5.0 */ public void terminate() { - if (getProvider().isRuntimeInitialized()) getProvider().getRuntime().tearDown(false); - getProvider().terminate(); + LocalContextProvider provider = getProvider(); + if (provider.isRuntimeInitialized()) { + provider.getRuntime().tearDown(false); + } + provider.terminate(); } /** @@ -1908,6 +1912,7 @@ public void terminate() { * * @since JRuby 1.6.0 */ + @Override public void finalize() throws Throwable { super.finalize(); terminate();