jruby · Feb 5, 2018 · Feb 5, 2018
diff --git a/core/src/main/java/org/jruby/javasupport/ext/JavaUtil.java b/core/src/main/java/org/jruby/javasupport/ext/JavaUtil.java
@@ -41,6 +41,7 @@
 
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
+import java.util.Collections;
 
 import static org.jruby.javasupport.JavaUtil.CAN_SET_ACCESSIBLE;
 import static org.jruby.javasupport.JavaUtil.convertJavaArrayToRuby;
@@ -226,8 +227,8 @@ public static IRubyObject op_minus(final ThreadContext context, final IRubyObjec
         @JRubyMethod
         public static IRubyObject dup(final ThreadContext context, final IRubyObject self) {
             java.util.Collection coll = unwrapIfJavaObject(self);
-            final JavaProxy dup = (JavaProxy) self.dup();
-            if ( coll == dup.getObject() ) { // not Cloneable
+            final JavaProxy dup = (JavaProxy) ((RubyBasicObject) self).dup();
+            if ( coll == dup.getObject() && ! (coll instanceof Cloneable) ) {
                 dup.setObject( tryNewEqualInstance(coll) );
             }
             return dup;
@@ -236,8 +237,8 @@ public static IRubyObject dup(final ThreadContext context, final IRubyObject sel
         @JRubyMethod
         public static IRubyObject clone(final ThreadContext context, final IRubyObject self) {
             java.util.Collection coll = unwrapIfJavaObject(self);
-            final JavaProxy dup = (JavaProxy) self.rbClone();
-            if ( coll == dup.getObject() ) { // not Cloneable
+            final JavaProxy dup = (JavaProxy) ((RubyBasicObject) self).rbClone();
+            if ( coll == dup.getObject() && ! (coll instanceof Cloneable) ) {
                 dup.setObject( tryNewEqualInstance(coll) );
             }
             return dup;
@@ -608,8 +609,10 @@ private static java.util.Collection tryNewEqualInstance(final java.util.Collecti
                     }
                 }
             }
-            if ( CAN_SET_ACCESSIBLE ) best.setAccessible(true);
-            return (java.util.Collection) best.newInstance(coll);
+            if ( best != null ) {
+                if ( CAN_SET_ACCESSIBLE ) best.setAccessible(true);
+                return (java.util.Collection) best.newInstance(coll);
+            }
         }
         catch (IllegalAccessException e) {
             // fallback on getConstructor();
@@ -621,18 +624,25 @@ private static java.util.Collection tryNewEqualInstance(final java.util.Collecti
             Helpers.throwException(e.getTargetException()); return null;
         }
 
+        final java.util.Collection clone;
         try {
-            java.util.Collection clone = klass.newInstance();
-            clone.addAll(coll);
-            return clone;
+            clone = klass.newInstance();
         }
         catch (IllegalAccessException e) {
             // can not clone - most of Collections. returned types (e.g. EMPTY_LIST)
             return coll;
         }
         catch (InstantiationException e) {
-            return coll;
+            Helpers.throwException(e); return null;
         }
+
+        //try {
+            clone.addAll(coll);
+        //}
+        //catch (UnsupportedOperationException|IllegalStateException e) {
+            // NOTE: maybe its better not mapping into a Ruby TypeError ?!
+        //}
+        return clone;
     }
 
 }
diff --git a/spec/java_integration/extensions/collection_spec.rb b/spec/java_integration/extensions/collection_spec.rb
@@ -139,6 +139,25 @@
     expect( arr.dup ).to be_a java.util.concurrent.LinkedBlockingQueue
     expect( arr.dup.poll ).to eq 42
     expect( arr.to_a ).to eql [42]
+
+    # immutable and Cloneable
+    arr = Java::java_integration::fixtures::coll::CloneableImmutableList.new
+    expect( arr.dup ).to equal arr
+
+    # non-Cloneable with public <init>()
+    arr = Java::java_integration::fixtures::coll::NonCloneableList.new; arr.add(42)
+    expect( arr.dup ).to eql arr
+    expect( arr.dup ).to_not equal arr
+
+    # immutable and non-Cloneable
+    arr = Java::java_integration::fixtures::coll::NonCloneableImmutableList.new
+    expect { arr.dup }.to raise_error # Java::JavaLang::UnsupportedOperationException
+
+    arr = Java::java_integration::fixtures::coll::NonCloneableImmutableList2::INSTANCE
+    expect { arr.dup }.to raise_error(Java::JavaLang::IllegalStateException) # since 9.2 (swallowed previously)
+
+    arr = Java::java_integration::fixtures::coll::NonCloneableImmutableList3.new
+    expect { arr.dup }.to raise_error(Java::JavaLang::UnsupportedOperationException) # not CloneNotSupportedException
   end
 
   it 'clones' do
@@ -157,6 +176,19 @@
     expect( set.clone ).to be_a java.util.concurrent.CopyOnWriteArraySet
     set.clone.add '1'
     expect( set.to_a ).to eql ['0']
+
+    # immutable and Cloneable
+    arr = Java::java_integration::fixtures::coll::CloneableImmutableList.new
+    expect( arr.clone ).to equal arr
+
+    # non-Cloneable with public <init>()
+    arr = Java::java_integration::fixtures::coll::NonCloneableList.new; arr.add(42)
+    expect( arr.clone ).to eql arr
+    expect( arr.clone ).to_not equal arr
+
+    # immutable and non-Cloneable
+    arr = Java::java_integration::fixtures::coll::NonCloneableImmutableList.new
+    expect { arr.clone }.to raise_error # Java::JavaLang::UnsupportedOperationException
   end
 
   it '#include?' do

diff --git a/spec/java_integration/fixtures/coll/CloneableImmutableList.java b/spec/java_integration/fixtures/coll/CloneableImmutableList.java
@@ -0,0 +1,22 @@
+package java_integration.fixtures.coll;
+
+import java.util.*;
+
+public final class CloneableImmutableList extends AbstractList<Object> implements Cloneable {
+    private final List<Object> storage = Arrays.asList(1, 2, 3);
+
+    @Override
+    public int size() {
+        return storage.size();
+    }
+
+    @Override
+    public Object get(int index) {
+        return storage.get(index);
+    }
+
+    @Override
+    public Object clone() {
+        return this;
+    }
+}
diff --git a/spec/java_integration/fixtures/coll/NonCloneableImmutableList.java b/spec/java_integration/fixtures/coll/NonCloneableImmutableList.java
@@ -0,0 +1,17 @@
+package java_integration.fixtures.coll;
+
+import java.util.*;
+
+public final class NonCloneableImmutableList extends AbstractList<Object> {
+    private final List<Object> storage = Arrays.asList(1, 2, 3);
+
+    @Override
+    public int size() {
+        return storage.size();
+    }
+
+    @Override
+    public Object get(int index) {
+        return storage.get(index);
+    }
+}
diff --git a/spec/java_integration/fixtures/coll/NonCloneableImmutableList2.java b/spec/java_integration/fixtures/coll/NonCloneableImmutableList2.java
@@ -0,0 +1,28 @@
+package java_integration.fixtures.coll;
+
+import java.util.*;
+
+public final class NonCloneableImmutableList2 extends AbstractList<Object> {
+
+    public static final NonCloneableImmutableList2 INSTANCE = new NonCloneableImmutableList2(true);
+
+    private final List<Object> storage = Arrays.asList(1);
+
+    public NonCloneableImmutableList2() {
+        throw new IllegalStateException("no instances");
+    }
+
+    private NonCloneableImmutableList2(boolean hidden) {
+        // no-op
+    }
+
+    @Override
+    public int size() {
+        return storage.size();
+    }
+
+    @Override
+    public Object get(int index) {
+        return storage.get(index);
+    }
+}
diff --git a/spec/java_integration/fixtures/coll/NonCloneableImmutableList3.java b/spec/java_integration/fixtures/coll/NonCloneableImmutableList3.java
@@ -0,0 +1,26 @@
+package java_integration.fixtures.coll;
+
+import java.util.*;
+
+public final class NonCloneableImmutableList3 extends AbstractList<Object> {
+
+    public static final NonCloneableImmutableList3 INSTANCE = new NonCloneableImmutableList3();
+
+    private final List<Object> storage = Arrays.asList(1);
+
+    @Override
+    public int size() {
+        return storage.size();
+    }
+
+    @Override
+    public Object get(int index) {
+        return storage.get(index);
+    }
+
+    @Override
+    public Object clone() throws CloneNotSupportedException {
+        throw new CloneNotSupportedException("no instances"); // ignored since Cloneable not implemented
+    }
+
+}
diff --git a/spec/java_integration/fixtures/coll/NonCloneableList.java b/spec/java_integration/fixtures/coll/NonCloneableList.java
@@ -0,0 +1,27 @@
+package java_integration.fixtures.coll;
+
+import java.util.*;
+
+public class NonCloneableList extends AbstractList<Object> {
+
+    final List<Object> storage = new ArrayList();
+
+    public NonCloneableList() {} // newInstance + addAll works for dup
+
+    @Override
+    public int size() {
+        return storage.size();
+    }
+
+    @Override
+    public Object get(int index) {
+        return storage.get(index);
+    }
+
+    @Override
+    public boolean add(Object e) {
+        storage.add(e);
+        return true;
+    }
+
+}