1

I have the following Comparator implemenation:

private static final class ValueComparator<Value_, Point_ extends Comparable<Point_>> implements Comparator<Value_> {

    private final Function<Value_, Point_> indexFunction;

    public ValueComparator(Function<Value_, Point_> indexFunction) {
        this.indexFunction = Objects.requireNonNull(indexFunction);
    }

    @Override
    public int compare(Value_ o1, Value_ o2) {
        if (o1 == o2) {
            return 0;
        }
        Point_ point1 = indexFunction.apply(o1);
        Point_ point2 = indexFunction.apply(o2);
        if (point1 == point2) {
            return 0;
        }
        int comparison = point1.compareTo(point2);
        if (comparison != 0) {
            return comparison;
        }
        // Identity Hashcode for duplicate protection; we must always include duplicates.
        // Ex: two different games on the same time slot
        int identityHashCode1 = System.identityHashCode(o1);
        int identityHashCode2 = System.identityHashCode(o2);
        return Integer.compare(identityHashCode1, identityHashCode2);
    }
}

This code runs for a while, and then suddenly throws an NPE - indexFunction somehow becomes null, even though it was checked to be non-null in the constructor.

Obviously, this was bugging me, so I went looking, and I found that the indexFunction is Math::abs, a reference to a static method. And java.lang.Math.abs(int) is @HotSpotIntrinsicCandidate.

Does that mean that HotSpot has, at some point, intrinsified the method and therefore the method reference became invalid? How do I protect myself against this sort of thing?

$ java -version
openjdk version "11.0.12" 2021-07-20
OpenJDK Runtime Environment Temurin-11.0.12+7 (build 11.0.12+7)
OpenJDK 64-Bit Server VM Temurin-11.0.12+7 (build 11.0.12+7, mixed mode)
Lukáš Petrovický
  • 3,945
  • 1
  • 11
  • 20

2 Answers2

2

Turns out the answer is much simpler and much less sensational. The argument to the indexFunction was null, and with the function expecting int, the NPE came from unboxing. In this case, the NPE still points to the indexFunction, as opposed to some code inside of it, which initially led me down a totally wrong path.

Apparently I wasn't as thorough in checking my inputs as I had thought. Rookie mistake, case closed.

Lukáš Petrovický
  • 3,945
  • 1
  • 11
  • 20
-1

You ran into a bug, saw something you didn't understand, and, evidently at your wit's end and not capable of understanding how your code can be to blame, zoomed in on this thing you didn't understand: That must be it!

Generally that's not a great way to debug. The odds that a Java bug or design idiocy explains your problem is infinitesimal (because let's be honest, if final variables randomly disappeared into thin air and either became null somehow, or, worse, remained whatever they were, but any attempt to interact with it results in a 'oh the function disappeared', and that is handled by, of all things, a NullPointerException - that's idiotic). That's one of the advantages of using a language with this many users. Odds are very low you're the first to run into it.

I'd know, I lost a Google Code Jam challenge once because of a now confirmed JVM bug. So it can happen. But the odds are infinitesimal.

Does that mean that HotSpot has, at some point, intrinsified the method and therefore the method reference became invalid?

No. That's not possible. That's not what 'intrinsic' does or means.

How do I protect myself against this sort of thing?

You can't, which is good because you don't have to - that never happens.

Check your stack trace. indexFunction isn't null. However, the index function may itself throw NPE, which it probably would, if o1 is null and o2 is not or vice versa.

On a separate note, why did you write this code in the first place? This already exists in the JVM:

instead of calling:

new ValueComparator(x -> x.getPoint());

you can simply call:

Comparator.comparing(x -> x.getPoint()).thenComparingInt(System::identityHashCode);
halfer
  • 19,824
  • 17
  • 99
  • 186
rzwitserloot
  • 85,357
  • 5
  • 51
  • 72
  • The reason why I wrote that code is because this code is on the hot path and your nice comparator has lambdas in it that I was trying to avoid. I was going to benchmark this hand-rolled comparator to see how it performs against the one you provided. – Lukáš Petrovický Aug 18 '21 at 14:13
  • And I think I found out what actually happened. The `indexFunction` got a null argument and the NPE pointed to the function being null, not to the contents of it. That makes sense, if the function was intrinsified - there is nowhere inside the function that it could point to. – Lukáš Petrovický Aug 18 '21 at 14:14
  • @LukasPetrovicky if you're using `Math.abs(int)`, isn't the NPE on a null argument coming from the unboxing, i.e. nothing to do with the function itself? – Andy Turner Aug 18 '21 at 14:44
  • @AndyTurner Yes, it is. I should probably only write things down after I'm absolutely sure what happened. Lesson learned. – Lukáš Petrovický Aug 18 '21 at 14:52