eclipse / eclipse-collections

Eclipse Collections is a collections framework for Java with optimized data structures and a rich, functional and fluent API.
http://www.eclipse.org/collections
2.42k stars 604 forks source link

Investigate whether Collectors2AdditionalTest.sumBy*Parallel is thread-safe. #587

Open motlin opened 6 years ago

motlin commented 6 years ago

I saw this one-off error in Travis and I don't want to lose track of it.

13:38:40:796 [ERROR] Tests run: 64, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 1.594 s <<< FAILURE! - in org.eclipse.collections.impl.collector.Collectors2AdditionalTest
13:38:40:796 [ERROR] sumByDoubleParallel(org.eclipse.collections.impl.collector.Collectors2AdditionalTest)  Time elapsed: 0.036 s  <<< ERROR!
java.lang.ArrayIndexOutOfBoundsException
    at org.eclipse.collections.impl.collector.Collectors2AdditionalTest.sumByDoubleParallel(Collectors2AdditionalTest.java:225)
Caused by: java.lang.ArrayIndexOutOfBoundsException: 1
13:39:23:932 [ERROR] Errors: 
13:39:23:933 [ERROR]   Collectors2AdditionalTest.sumByDoubleParallel:225 » ArrayIndexOutOfBounds
13:39:23:933 [INFO] 
13:39:23:933 [ERROR] Tests run: 149942, Failures: 0, Errors: 1, Skipped: 0
nikhilnanivadekar commented 6 years ago

I had noticed this one before as well. I disqualified it as one off but I guess it's persistent. I was not able to replicate it in local to actually fix it

motlin commented 2 months ago

I saw this again today @donraab @nikhilnanivadekar

I finally looked through the code and it's not thread-safe in a straight-forward way. I'd just delete the test because it makes no sense but I'd like to get your opinions on the rationale behind the test.

    @Test
    public void sumByDoubleParallel()
    {
        MutableList<Double> largeLongs = LARGE_INTERVAL.collect(Double::valueOf).toList();
        assertEquals(
                largeLongs.sumByDouble(each -> Integer.valueOf(each.intValue() % 2), Double::doubleValue),
                largeLongs.parallelStream().collect(Collectors2.sumByDouble(each -> Integer.valueOf(each.intValue() % 2), Double::doubleValue)));
    }
org.eclipse.collections.impl.collector.Collectors2AdditionalTest.sumByDoubleParallel -- Time elapsed: 0.021 s <<< ERROR!
java.lang.ArrayIndexOutOfBoundsException: Index 1 out of bounds for length 1
    at org.eclipse.collections.impl.map.mutable.primitive.ObjectDoubleHashMap.addKeyValueAtIndex(ObjectDoubleHashMap.java:583)
    at org.eclipse.collections.impl.map.mutable.primitive.ObjectDoubleHashMap.put(ObjectDoubleHashMap.java:435)
    at org.eclipse.collections.impl.block.factory.PrimitiveFunctions$4.value(PrimitiveFunctions.java:182)
    at org.eclipse.collections.impl.block.factory.PrimitiveFunctions$4.value(PrimitiveFunctions.java:171)
    at java.base/java.util.stream.ReduceOps$3ReducingSink.accept(ReduceOps.java:169)
    at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:948)
    at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484)
    at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
    at java.base/java.util.stream.ReduceOps$ReduceTask.doLeaf(ReduceOps.java:952)
    at java.base/java.util.stream.ReduceOps$ReduceTask.doLeaf(ReduceOps.java:926)
    at java.base/java.util.stream.AbstractTask.compute(AbstractTask.java:327)
    at java.base/java.util.concurrent.CountedCompleter.exec(CountedCompleter.java:746)
    at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290)
    at java.base/java.util.concurrent.ForkJoinTask.doInvoke(ForkJoinTask.java:408)
    at java.base/java.util.concurrent.ForkJoinTask.invoke(ForkJoinTask.java:736)
    at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateParallel(ReduceOps.java:919)
    at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:233)
    at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:578)
    at org.eclipse.collections.impl.collector.Collectors2AdditionalTest.sumByDoubleParallel(Collectors2AdditionalTest.java:226)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)