From 7fb572ba724f7c3d0ded240783ac722ae000bdcc Mon Sep 17 00:00:00 2001 From: psandoz Date: Thu, 1 Aug 2013 15:28:57 +0100 Subject: [PATCH] 8020016: Numerous splitereator impls do not throw NPE for null Consumers Reviewed-by: mduigou, alanb, henryjen --- .../java/util/stream/SpinedBuffer.java | 9 ++++ .../java/util/stream/StreamSpliterators.java | 33 ++++++++++++++ .../classes/java/util/stream/Streams.java | 25 +++++++++++ ...SpliteratorTraversingAndSplittingTest.java | 25 ++++++++++- .../util/stream/SpliteratorTestHelper.java | 45 +++++++++++++++++++ 5 files changed, 136 insertions(+), 1 deletion(-) diff --git a/src/share/classes/java/util/stream/SpinedBuffer.java b/src/share/classes/java/util/stream/SpinedBuffer.java index 77c375612..7312c984a 100644 --- a/src/share/classes/java/util/stream/SpinedBuffer.java +++ b/src/share/classes/java/util/stream/SpinedBuffer.java @@ -28,6 +28,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Iterator; import java.util.List; +import java.util.Objects; import java.util.PrimitiveIterator; import java.util.Spliterator; import java.util.Spliterators; @@ -317,6 +318,8 @@ class SpinedBuffer @Override public boolean tryAdvance(Consumer consumer) { + Objects.requireNonNull(consumer); + if (splSpineIndex < lastSpineIndex || (splSpineIndex == lastSpineIndex && splElementIndex < lastSpineElementFence)) { consumer.accept(splChunk[splElementIndex++]); @@ -334,6 +337,8 @@ class SpinedBuffer @Override public void forEachRemaining(Consumer consumer) { + Objects.requireNonNull(consumer); + if (splSpineIndex < lastSpineIndex || (splSpineIndex == lastSpineIndex && splElementIndex < lastSpineElementFence)) { int i = splElementIndex; @@ -634,6 +639,8 @@ class SpinedBuffer @Override public boolean tryAdvance(T_CONS consumer) { + Objects.requireNonNull(consumer); + if (splSpineIndex < lastSpineIndex || (splSpineIndex == lastSpineIndex && splElementIndex < lastSpineElementFence)) { arrayForOne(splChunk, splElementIndex++, consumer); @@ -651,6 +658,8 @@ class SpinedBuffer @Override public void forEachRemaining(T_CONS consumer) { + Objects.requireNonNull(consumer); + if (splSpineIndex < lastSpineIndex || (splSpineIndex == lastSpineIndex && splElementIndex < lastSpineElementFence)) { int i = splElementIndex; diff --git a/src/share/classes/java/util/stream/StreamSpliterators.java b/src/share/classes/java/util/stream/StreamSpliterators.java index d0dc61ed1..1a27e98ff 100644 --- a/src/share/classes/java/util/stream/StreamSpliterators.java +++ b/src/share/classes/java/util/stream/StreamSpliterators.java @@ -25,6 +25,7 @@ package java.util.stream; import java.util.Comparator; +import java.util.Objects; import java.util.Spliterator; import java.util.concurrent.atomic.AtomicLong; import java.util.function.BooleanSupplier; @@ -294,6 +295,7 @@ class StreamSpliterators { @Override public boolean tryAdvance(Consumer consumer) { + Objects.requireNonNull(consumer); boolean hasNext = doAdvance(); if (hasNext) consumer.accept(buffer.get(nextToConsume)); @@ -303,6 +305,7 @@ class StreamSpliterators { @Override public void forEachRemaining(Consumer consumer) { if (buffer == null && !finished) { + Objects.requireNonNull(consumer); init(); ph.wrapAndCopyInto((Sink) consumer::accept, spliterator); @@ -350,6 +353,7 @@ class StreamSpliterators { @Override public boolean tryAdvance(IntConsumer consumer) { + Objects.requireNonNull(consumer); boolean hasNext = doAdvance(); if (hasNext) consumer.accept(buffer.get(nextToConsume)); @@ -359,6 +363,7 @@ class StreamSpliterators { @Override public void forEachRemaining(IntConsumer consumer) { if (buffer == null && !finished) { + Objects.requireNonNull(consumer); init(); ph.wrapAndCopyInto((Sink.OfInt) consumer::accept, spliterator); @@ -406,6 +411,7 @@ class StreamSpliterators { @Override public boolean tryAdvance(LongConsumer consumer) { + Objects.requireNonNull(consumer); boolean hasNext = doAdvance(); if (hasNext) consumer.accept(buffer.get(nextToConsume)); @@ -415,6 +421,7 @@ class StreamSpliterators { @Override public void forEachRemaining(LongConsumer consumer) { if (buffer == null && !finished) { + Objects.requireNonNull(consumer); init(); ph.wrapAndCopyInto((Sink.OfLong) consumer::accept, spliterator); @@ -462,6 +469,7 @@ class StreamSpliterators { @Override public boolean tryAdvance(DoubleConsumer consumer) { + Objects.requireNonNull(consumer); boolean hasNext = doAdvance(); if (hasNext) consumer.accept(buffer.get(nextToConsume)); @@ -471,6 +479,7 @@ class StreamSpliterators { @Override public void forEachRemaining(DoubleConsumer consumer) { if (buffer == null && !finished) { + Objects.requireNonNull(consumer); init(); ph.wrapAndCopyInto((Sink.OfDouble) consumer::accept, spliterator); @@ -696,6 +705,8 @@ class StreamSpliterators { @Override public boolean tryAdvance(Consumer action) { + Objects.requireNonNull(action); + if (sliceOrigin >= fence) return false; @@ -713,6 +724,8 @@ class StreamSpliterators { @Override public void forEachRemaining(Consumer action) { + Objects.requireNonNull(action); + if (sliceOrigin >= fence) return; @@ -754,6 +767,8 @@ class StreamSpliterators { @Override public boolean tryAdvance(T_CONS action) { + Objects.requireNonNull(action); + if (sliceOrigin >= fence) return false; @@ -771,6 +786,8 @@ class StreamSpliterators { @Override public void forEachRemaining(T_CONS action) { + Objects.requireNonNull(action); + if (sliceOrigin >= fence) return; @@ -985,6 +1002,8 @@ class StreamSpliterators { @Override public boolean tryAdvance(Consumer action) { + Objects.requireNonNull(action); + while (permitStatus() != PermitStatus.NO_MORE) { if (!s.tryAdvance(this)) return false; @@ -999,6 +1018,8 @@ class StreamSpliterators { @Override public void forEachRemaining(Consumer action) { + Objects.requireNonNull(action); + ArrayBuffer.OfRef sb = null; PermitStatus permitStatus; while ((permitStatus = permitStatus()) != PermitStatus.NO_MORE) { @@ -1051,6 +1072,8 @@ class StreamSpliterators { @Override public boolean tryAdvance(T_CONS action) { + Objects.requireNonNull(action); + while (permitStatus() != PermitStatus.NO_MORE) { if (!s.tryAdvance((T_CONS) this)) return false; @@ -1066,6 +1089,8 @@ class StreamSpliterators { @Override public void forEachRemaining(T_CONS action) { + Objects.requireNonNull(action); + T_BUFF sb = null; PermitStatus permitStatus; while ((permitStatus = permitStatus()) != PermitStatus.NO_MORE) { @@ -1237,6 +1262,8 @@ class StreamSpliterators { @Override public boolean tryAdvance(Consumer action) { + Objects.requireNonNull(action); + action.accept(s.get()); return true; } @@ -1260,6 +1287,8 @@ class StreamSpliterators { @Override public boolean tryAdvance(IntConsumer action) { + Objects.requireNonNull(action); + action.accept(s.getAsInt()); return true; } @@ -1283,6 +1312,8 @@ class StreamSpliterators { @Override public boolean tryAdvance(LongConsumer action) { + Objects.requireNonNull(action); + action.accept(s.getAsLong()); return true; } @@ -1306,6 +1337,8 @@ class StreamSpliterators { @Override public boolean tryAdvance(DoubleConsumer action) { + Objects.requireNonNull(action); + action.accept(s.getAsDouble()); return true; } diff --git a/src/share/classes/java/util/stream/Streams.java b/src/share/classes/java/util/stream/Streams.java index fe83b9708..15c3dcae4 100644 --- a/src/share/classes/java/util/stream/Streams.java +++ b/src/share/classes/java/util/stream/Streams.java @@ -25,6 +25,7 @@ package java.util.stream; import java.util.Comparator; +import java.util.Objects; import java.util.Spliterator; import java.util.function.Consumer; import java.util.function.DoubleConsumer; @@ -80,6 +81,8 @@ final class Streams { @Override public boolean tryAdvance(IntConsumer consumer) { + Objects.requireNonNull(consumer); + final int i = from; if (i < upTo) { from++; @@ -96,6 +99,8 @@ final class Streams { @Override public void forEachRemaining(IntConsumer consumer) { + Objects.requireNonNull(consumer); + int i = from; final int hUpTo = upTo; int hLast = last; @@ -199,6 +204,8 @@ final class Streams { @Override public boolean tryAdvance(LongConsumer consumer) { + Objects.requireNonNull(consumer); + final long i = from; if (i < upTo) { from++; @@ -215,6 +222,8 @@ final class Streams { @Override public void forEachRemaining(LongConsumer consumer) { + Objects.requireNonNull(consumer); + long i = from; final long hUpTo = upTo; int hLast = last; @@ -388,6 +397,8 @@ final class Streams { @Override public boolean tryAdvance(Consumer action) { + Objects.requireNonNull(action); + if (count == -2) { action.accept(first); count = -1; @@ -400,6 +411,8 @@ final class Streams { @Override public void forEachRemaining(Consumer action) { + Objects.requireNonNull(action); + if (count == -2) { action.accept(first); count = -1; @@ -475,6 +488,8 @@ final class Streams { @Override public boolean tryAdvance(IntConsumer action) { + Objects.requireNonNull(action); + if (count == -2) { action.accept(first); count = -1; @@ -487,6 +502,8 @@ final class Streams { @Override public void forEachRemaining(IntConsumer action) { + Objects.requireNonNull(action); + if (count == -2) { action.accept(first); count = -1; @@ -562,6 +579,8 @@ final class Streams { @Override public boolean tryAdvance(LongConsumer action) { + Objects.requireNonNull(action); + if (count == -2) { action.accept(first); count = -1; @@ -574,6 +593,8 @@ final class Streams { @Override public void forEachRemaining(LongConsumer action) { + Objects.requireNonNull(action); + if (count == -2) { action.accept(first); count = -1; @@ -649,6 +670,8 @@ final class Streams { @Override public boolean tryAdvance(DoubleConsumer action) { + Objects.requireNonNull(action); + if (count == -2) { action.accept(first); count = -1; @@ -661,6 +684,8 @@ final class Streams { @Override public void forEachRemaining(DoubleConsumer action) { + Objects.requireNonNull(action); + if (count == -2) { action.accept(first); count = -1; diff --git a/test/java/util/Spliterator/SpliteratorTraversingAndSplittingTest.java b/test/java/util/Spliterator/SpliteratorTraversingAndSplittingTest.java index 6da6214e1..458c410c2 100644 --- a/test/java/util/Spliterator/SpliteratorTraversingAndSplittingTest.java +++ b/test/java/util/Spliterator/SpliteratorTraversingAndSplittingTest.java @@ -81,6 +81,10 @@ import java.util.function.UnaryOperator; import static org.testng.Assert.*; import static org.testng.Assert.assertEquals; +/** + * @test + * @bug 8020016 + */ @Test public class SpliteratorTraversingAndSplittingTest { @@ -386,11 +390,23 @@ public class SpliteratorTraversingAndSplittingTest { db.addCollection(CopyOnWriteArraySet::new); - if (size == 1) { + if (size == 0) { + db.addCollection(c -> Collections.emptySet()); + db.addList(c -> Collections.emptyList()); + } + else if (size == 1) { db.addCollection(c -> Collections.singleton(exp.get(0))); db.addCollection(c -> Collections.singletonList(exp.get(0))); } + { + Integer[] ai = new Integer[size]; + Arrays.fill(ai, 1); + db.add(String.format("Collections.nCopies(%d, 1)", exp.size()), + Arrays.asList(ai), + () -> Collections.nCopies(exp.size(), 1).spliterator()); + } + // Collections.synchronized/unmodifiable/checked wrappers db.addCollection(Collections::unmodifiableCollection); db.addCollection(c -> Collections.unmodifiableSet(new HashSet<>(c))); @@ -454,6 +470,13 @@ public class SpliteratorTraversingAndSplittingTest { db.addMap(ConcurrentHashMap::new); db.addMap(ConcurrentSkipListMap::new); + + if (size == 0) { + db.addMap(m -> Collections.emptyMap()); + } + else if (size == 1) { + db.addMap(m -> Collections.singletonMap(exp.get(0), exp.get(0))); + } } return spliteratorDataProvider = data.toArray(new Object[0][]); diff --git a/test/java/util/stream/bootlib/java/util/stream/SpliteratorTestHelper.java b/test/java/util/stream/bootlib/java/util/stream/SpliteratorTestHelper.java index 4d2217353..a3446ffd6 100644 --- a/test/java/util/stream/bootlib/java/util/stream/SpliteratorTestHelper.java +++ b/test/java/util/stream/bootlib/java/util/stream/SpliteratorTestHelper.java @@ -22,6 +22,8 @@ */ package java.util.stream; +import org.testng.annotations.Test; + import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Collection; @@ -154,6 +156,7 @@ public class SpliteratorTestHelper { Collection exp = Collections.unmodifiableList(fromForEach); + testNullPointerException(supplier); testForEach(exp, supplier, boxingAdapter, asserter); testTryAdvance(exp, supplier, boxingAdapter, asserter); testMixedTryAdvanceForEach(exp, supplier, boxingAdapter, asserter); @@ -166,6 +169,31 @@ public class SpliteratorTestHelper { // + private static > void testNullPointerException(Supplier s) { + S sp = s.get(); + // Have to check instances and use casts to avoid tripwire messages and + // directly test the primitive methods + if (sp instanceof Spliterator.OfInt) { + Spliterator.OfInt psp = (Spliterator.OfInt) sp; + executeAndCatch(NullPointerException.class, () -> psp.forEachRemaining((IntConsumer) null)); + executeAndCatch(NullPointerException.class, () -> psp.tryAdvance((IntConsumer) null)); + } + else if (sp instanceof Spliterator.OfLong) { + Spliterator.OfLong psp = (Spliterator.OfLong) sp; + executeAndCatch(NullPointerException.class, () -> psp.forEachRemaining((LongConsumer) null)); + executeAndCatch(NullPointerException.class, () -> psp.tryAdvance((LongConsumer) null)); + } + else if (sp instanceof Spliterator.OfDouble) { + Spliterator.OfDouble psp = (Spliterator.OfDouble) sp; + executeAndCatch(NullPointerException.class, () -> psp.forEachRemaining((DoubleConsumer) null)); + executeAndCatch(NullPointerException.class, () -> psp.tryAdvance((DoubleConsumer) null)); + } + else { + executeAndCatch(NullPointerException.class, () -> sp.forEachRemaining(null)); + executeAndCatch(NullPointerException.class, () -> sp.tryAdvance(null)); + } + } + private static > void testForEach( Collection exp, Supplier supplier, @@ -573,6 +601,23 @@ public class SpliteratorTestHelper { } } + private static void executeAndCatch(Class expected, Runnable r) { + Exception caught = null; + try { + r.run(); + } + catch (Exception e) { + caught = e; + } + + assertNotNull(caught, + String.format("No Exception was thrown, expected an Exception of %s to be thrown", + expected.getName())); + assertTrue(expected.isInstance(caught), + String.format("Exception thrown %s not an instance of %s", + caught.getClass().getName(), expected.getName())); + } + static void mixedTraverseAndSplit(Consumer b, Spliterator splTop) { Spliterator spl1, spl2, spl3; splTop.tryAdvance(b); -- GitLab