Fixes for iterative methods when adding elts

We can't assume the size will remain the same throughout the loop,
so these all need to check it each iteration. This allows items to
be added during the loop and still get picked up. Fixes a handful
of rubyspec tags.

core/src/main/java/org/jruby/RubyArray.java

@@ -2243,30 +2243,29 @@ public IRubyObject to_h(ThreadContext context) {
     @JRubyMethod(name = "to_h")
     public IRubyObject to_h(ThreadContext context, Block block) {
         Ruby runtime = context.runtime;
-        int realLength = this.realLength;
 
         boolean useSmallHash = realLength <= 10;
 
         RubyHash hash = useSmallHash ? RubyHash.newSmallHash(runtime) : RubyHash.newHash(runtime);
 
         for (int i = 0; i < realLength; i++) {
-            IRubyObject e = eltInternal(i);
+            IRubyObject e = eltOk(i);
             IRubyObject elt = block.isGiven() ? block.yield(context, e) : e;
             IRubyObject key_value_pair = elt.checkArrayType();
 
             if (key_value_pair == context.nil) {
-                throw context.runtime.newTypeError("wrong element type " + elt.getMetaClass().getRealClass() + " at " + i + " (expected array)");
+                throw runtime.newTypeError("wrong element type " + elt.getMetaClass().getRealClass() + " at " + i + " (expected array)");
             }
 
             RubyArray ary = (RubyArray)key_value_pair;
             if (ary.getLength() != 2) {
-                throw context.runtime.newArgumentError("wrong array length at " + i + " (expected 2, was " + ary.getLength() + ")");
+                throw runtime.newArgumentError("wrong array length at " + i + " (expected 2, was " + ary.getLength() + ")");
             }
 
             if (useSmallHash) {
-                hash.fastASetSmall(runtime, ary.eltInternal(0), ary.eltInternal(1), true);
+                hash.fastASetSmall(runtime, ary.eltOk(0), ary.eltOk(1), true);
             } else {
-                hash.fastASet(runtime, ary.eltInternal(0), ary.eltInternal(1), true);
+                hash.fastASet(runtime, ary.eltOk(0), ary.eltOk(1), true);
             }
         }
         return hash;
@@ -2770,17 +2769,16 @@ public RubyArray collectArray(ThreadContext context, Block block) {
 
         final Ruby runtime = context.runtime;
 
-        IRubyObject[] arr = IRubyObject.array(realLength);
+        RubyArray ary = RubyArray.newArray(runtime, realLength);
 
         int i = 0;
         for (; i < realLength; i++) {
             // Do not coarsen the "safe" check, since it will misinterpret AIOOBE from the yield
             // See JRUBY-5434
-            safeArraySet(runtime, arr, i, block.yieldNonArray(context, eltOk(i), null)); // arr[i] = ...
+            ary.store(i, block.yieldNonArray(context, eltOk(i), null)); // arr[i] = ...
         }
 
-        // use iteration count as new size in case something was deleted along the way
-        return newArrayMayCopy(context.runtime, arr, 0, i);
+        return ary;
     }
 
     /**
@@ -2820,7 +2818,7 @@ public RubyArray collectBang(ThreadContext context, Block block) {
         if (!block.isGiven()) throw context.runtime.newLocalJumpErrorNoBlock();
         modify();
 
-        for (int i = 0, len = realLength; i < len; i++) {
+        for (int i = 0; i < realLength; i++) {
             // Do not coarsen the "safe" check, since it will misinterpret AIOOBE from the yield
             // See JRUBY-5434
             storeInternal(i, block.yield(context, eltOk(i)));
@@ -2901,23 +2899,21 @@ public IRubyObject select_bang(ThreadContext context, Block block) {
 
         boolean modified = false;
         final Ruby runtime = context.runtime;
-        final int len = realLength; final int beg = begin;
 
         int len0 = 0, len1 = 0;
         try {
             int i1, i2;
-            for (i1 = i2 = 0; i1 < len; len0 = ++i1) {
-                final IRubyObject[] values = this.values;
+            for (i1 = i2 = 0; i1 < realLength; len0 = ++i1) {
                 // Do not coarsen the "safe" check, since it will misinterpret
                 // AIOOBE from the yield (see JRUBY-5434)
-                IRubyObject value = safeArrayRef(runtime, values, begin + i1);
+                IRubyObject value = eltOk(i1);
 
                 if (!block.yield(context, value).isTrue()) {
                     modified = true;
                     continue;
                 }
 
-                if (i1 != i2) safeArraySet(runtime, values, beg + i2, value);
+                if (i1 != i2) eltSetOk(i2, (T) value);
                 len1 = ++i2;
             }
             return (i1 == i2) ? context.nil : this;
@@ -3038,8 +3034,14 @@ public IRubyObject delete_at(IRubyObject obj) {
      *
      */
     public final IRubyObject rejectCommon(ThreadContext context, Block block) {
-        RubyArray ary = aryDup();
-        ary.rejectBang(context, block);
+        RubyArray ary = RubyArray.newArray(context.runtime);
+
+        for (int i = 0; i < realLength; i++) {
+            IRubyObject v = eltOk(i);
+            if (!block.yieldSpecific(context, v).isTrue()) {
+                ary.push(v);
+            }
+        }
         return ary;
     }
 

core/src/main/java/org/jruby/RubyEnumerable.java

@@ -61,6 +61,7 @@
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.BiFunction;
@@ -496,55 +497,37 @@ public static IRubyObject sort(ThreadContext context, IRubyObject self, final Bl
     @JRubyMethod
     public static IRubyObject sort_by(final ThreadContext context, IRubyObject self, final Block block) {
         final Ruby runtime = context.runtime;
-        DoubleObject<IRubyObject, IRubyObject>[] valuesAndCriteria;
+        List<DoubleObject<IRubyObject, IRubyObject>> valuesAndCriteria;
 
         if (!block.isGiven()) {
             return enumeratorizeWithSize(context, self, "sort_by", (SizeFn) RubyEnumerable::size);
         }
 
         final CachingCallSite each = eachSite(context);
-        if (self instanceof RubyArray) {
-            RubyArray selfArray = (RubyArray) self;
-            final DoubleObject<IRubyObject, IRubyObject>[] valuesAndCriteriaArray = new DoubleObject[selfArray.size()];
+        final ArrayList<DoubleObject<IRubyObject, IRubyObject>> valuesAndCriteriaList = new ArrayList<>();
 
-            each(context, each, self, new JavaInternalBlockBody(runtime, Signature.OPTIONAL) {
-                final AtomicInteger i = new AtomicInteger(0);
-                @Override
-                public IRubyObject yield(ThreadContext context1, IRubyObject[] args) {
-                    return doYield(context1, null, packEnumValues(context1, args));
-                }
-                @Override
-                protected IRubyObject doYield(ThreadContext context1, Block unused, IRubyObject value) {
-                    valuesAndCriteriaArray[i.getAndIncrement()] = new DoubleObject<>(value, block.yield(context1, value));
-                    return context1.nil;
-                }
-            });
-
-            valuesAndCriteria = valuesAndCriteriaArray;
-        } else {
-            final ArrayList<DoubleObject<IRubyObject, IRubyObject>> valuesAndCriteriaList = new ArrayList<>();
-
-            callEach(context, each, self, Signature.OPTIONAL, new BlockCallback() {
-                public IRubyObject call(ThreadContext context1, IRubyObject[] args, Block unused) {
-                    return call(context1, packEnumValues(context1, args), unused);
-                }
-                @Override
-                public IRubyObject call(ThreadContext context1, IRubyObject arg, Block unused) {
-                    valuesAndCriteriaList.add(new DoubleObject<>(arg, block.yield(context1, arg)));
-                    return context1.nil;
+        callEach(context, each, self, Signature.OPTIONAL, new BlockCallback() {
+            public IRubyObject call(ThreadContext context1, IRubyObject[] args, Block unused) {
+                return call(context1, packEnumValues(context1, args), unused);
+            }
+            @Override
+            public IRubyObject call(ThreadContext context1, IRubyObject arg, Block unused) {
+                IRubyObject value = block.yield(context1, arg);
+                synchronized (valuesAndCriteriaList) {
+                    valuesAndCriteriaList.add(new DoubleObject<>(arg, value));
                 }
-            });
+                return context1.nil;
+            }
+        });
 
-            valuesAndCriteria = valuesAndCriteriaList.toArray(new DoubleObject[valuesAndCriteriaList.size()]);
-        }
+        valuesAndCriteria = valuesAndCriteriaList;
 
-        Arrays.sort(valuesAndCriteria,
+        Collections.sort(valuesAndCriteria,
                 (o1, o2) -> RubyComparable.cmpint(context, invokedynamic(context, o1.object2, OP_CMP, o2.object2), o1.object2, o2.object2));
 
-
-        IRubyObject dstArray[] = new IRubyObject[valuesAndCriteria.length];
+        IRubyObject dstArray[] = new IRubyObject[valuesAndCriteria.size()];
         for (int i = 0; i < dstArray.length; i++) {
-            dstArray[i] = valuesAndCriteria[i].object1;
+            dstArray[i] = valuesAndCriteria.get(i).object1;
         }
 
         return RubyArray.newArrayMayCopy(runtime, dstArray);