RFR(XS): 8174950: Gracefully handle null Supplier in Objects.requireNonNull

David Holmes david.holmes at oracle.com
Wed Feb 15 02:03:22 UTC 2017

Looks good to me!


On 15/02/2017 4:52 AM, Volker Simonis wrote:
> Hi,
> can I please have another review for the following trivial fix:
> http://cr.openjdk.java.net/~simonis/webrevs/2017/8174950/
> https://bugs.openjdk.java.net/browse/JDK-8174950
> This change has already been discussed in length on the mailing list:
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-February/thread.html#46324
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-February/thread.html#24989
> and in the bug comments at:
> https://bugs.openjdk.java.net/browse/JDK-8033909
> So I'll give just (yet another) short summary here:
> - Objects.requireNonNull(T, Supplier) does not check for the Supplier
> argument being null. Instead it relies on the fact, that the VM will
> implicitly throw a NullPointerException when it calls .get on the
> Supplier argument during the creation of the explicit
> NullPointerException which it is supposed to throw.
> - this behavior slightly differs from Objects.requireNonNull(T,
> String) which simply creates a NullPointerException with a null
> message in the case where the String argument is null.
> - the difference is not evident in the OpenJDK, because the HotSpot VM
> creates a NPE with a null message by default if we call a method on a
> null object.
> - however creating such a NPE with a null message when invoking a
> method on a null object is not enforced by the standard, and other
> implementations can do better :) For the following trivial program:
> public class NonNull {
>   public static void main(String[] args) {
>     Supplier<String> ss = null;
>     Object o = Objects.requireNonNull(null, ss);
>   }
> }
> the current OpenJDK implementation returns:
> Exception in thread "main" java.lang.NullPointerException
>     at java.util.Objects.requireNonNull(Objects.java:290)
>     at NonNull.main(NonNull.java:8)
> but the SAP JVM will print:
> Exception in thread "main" java.lang.NullPointerException: while
> trying to invoke the method java.util.function.Supplier.get() of a
> null object loaded from local variable 'messageSupplier'
>     at java.util.Objects.requireNonNull(Objects.java:290)
>     at NonNull.main(NonNull.java:8)
> which is at least confusing for a call to Objects.requireNonNul() with
> a null Supplier. We think the exception should be the same like the
> one we get when invoking Objects.requireNonNul() with a null String.
> - because of this difference (and because we like our extended
> Exception messages :) the JCK test for Objects.requireNonNul(T,
> Supplier) (i.e.
> api/java_util/Objects/index.html#RequireNonNullMessageSupplier) was
> removed from TCK 8 and is still commented out in TCK 9
> (npe_checkingNullSupplier() in RequireNonNullMessageSupplier.java).
> I really think that the simplest and most natural fix for this problem
> is to simply check for a null Supplier argument and create the NPE
> with an explicit null message in that case (as outlined in the
> webrev). This:
>  - makes the two requireNonNul() overloads for String and Supplier
> behave the same (which I think was the initial intention).
>  - doesn't require documentation changes
>  - makes it possible to write a simple and conforming TCK test
> Regards,
> Volker

More information about the core-libs-dev mailing list