On Thu, 17 Nov 2022 06:05:40 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Can I please get a review of this change which proposes to fix >> https://bugs.openjdk.org/browse/JDK-8292317? >> >> The `java.util.Iterator` has a `forEachRemaining(Consumer<? super E> >> action)` method. As per its contract, the implementations are expected to >> throw a `NullPointerException` if the passed `action` is `null`. >> >> `java.util.Collections` has a couple of places where it wraps the passed >> `action` into another and passes that wrapped `action` to the underlying >> `java.util.Iterator`'s default `forEachRemaining` implementation which is: >> >> >> Objects.requireNonNull(action); >> while (hasNext()) >> action.accept(next()); >> >> Since the passed `action` is now a non-null wrapper, the implementation goes >> ahead and advances the iterator to the next entry and invokes on the >> non-null `action` to carry out its action. That non-null wrapper action then >> calls the `null` user passed action and runs into an expected >> `NullPointerException`. However, at this point the iterator has already >> advanced and that is what the bug is. >> >> The commit in this PR introduces a trivial null check on the `action` very >> early in the call even before wrapping such an `action`. This prevents any >> further logic to execute if `action` is null. >> >> New test methods have been added to the existing test class >> `test/jdk/java/util/Collections/DelegatingIteratorForEachRemaining.java`. >> This test class was introduced in >> https://bugs.openjdk.org/browse/JDK-8205184 where this delegation logic was >> added for the `forEachRemaining` methods. These new test methods reproduce >> the failure and verify the fix. > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > review suggestion - simplify test tier1, tier2, tier3 and java_util tests in JCK have passed with this change. ------------- PR: https://git.openjdk.org/jdk/pull/11154
