Random IllegalArgumentException when importing Java types

[DavidPerezIngeniero]

When importing this class:

import scala.jdk.CollectionConverters

whose documentation is here:
https://www.scala-lang.org/api/2.13.3/scala/jdk/CollectionConverters$.html

the following happens radomly:

java.lang.IllegalArgumentException: Comparison method violates its general contract!
	at java.base/java.util.TimSort.mergeHi(TimSort.java:903)
	at java.base/java.util.TimSort.mergeAt(TimSort.java:520)
	at java.base/java.util.TimSort.mergeCollapse(TimSort.java:448)
	at java.base/java.util.TimSort.sort(TimSort.java:245)
	at java.base/java.util.Arrays.sort(Arrays.java:1233)
	at org.python.core.PyJavaType.init(PyJavaType.java:304)
	at org.python.core.PyType.createType(PyType.java:1414)

Here is tha failing code from file PyJavaType.java:

Arrays.sort(methods, new MethodComparator(new ClassComparator()));

After some research, I've come to the conclusion that the MethodComparator for sorting the methods of a Java class, doesn't have associative properties.

In the failing class, we have many methods with the same name and number of arguments but different signature.

private class MethodComparator implements Comparator<Method> {

        private ClassComparator classComparator;

        public MethodComparator(ClassComparator classComparator) {
            this.classComparator = classComparator;
        }

        public int compare(Method m1, Method m2) {
            int result = m1.getName().compareTo(m2.getName());

            if (result != 0) {
                return result;
            }

            Class<?>[] p1 = m1.getParameterTypes();
            Class<?>[] p2 = m2.getParameterTypes();

            int n1 = p1.length;
            int n2 = p2.length;

            result = n1 - n2;

            if (result != 0) {
                return result;
            }

            result = classComparator.compare(m1.getDeclaringClass(), m2.getDeclaringClass());

            if (result != 0) {
                return result;
            }

            if (n1 == 0) {
                return classComparator.compare(m1.getReturnType(), m2.getReturnType());
            } else if (n1 == 1) {
                return classComparator.compare(p1[0], p2[0]);
            }
            return result;
        }
    }

MethodComparator only considers:

• Defining class
• Method name
• Number of arguments
• Method return type

[jeff5]

Not sure what you mean by "associative properties", but I think your diagnosis could well be correct: it doesn't define a total order. Methods of the same name, class, number of arguments and return type, could still be different in signature, so not equal, while ranking equal in the sort.

[DavidPerezIngeniero]

Sorry, I've mistyped associative properties, I meant that the full signature of methods aren't used for comparison, only a subset, and there can be ambiguities.

[robertpatrick]

@DavidPerezIngeniero What do you mean by "the full signature of methods aren't used for comparison"?

Jython does its best to handle overloaded methods but there are definitely issues when using scalar types in such methods. For example, Jython has never really been able to differentiate between byte, char, short, int, and long.

[DavidPerezIngeniero]

I mean that only for comparison is used the name of the method, the class which it belongs to, the number of arguments, the return type, but not the type of each argument.

[robertpatrick]

@DavidPerezIngeniero That's simply not the case. It is looking at the argument types when matching methods (with the previous caveats around scalar types). Take a look at how the __call__ method of PyReflectedFunction works. PyReflectedConstructor.py has similar logic.

By the way, Java method selection never uses the return type. Try overloading a method where the return types are different and see what the Java compiler has to say about it...

[jeff5]

Well, @robertpatrick , @DavidPerezIngeniero quotes a specific comparator in PyJavaType as the cause of the problem.

It's different from the comparator you refer to involving ReflectedArgs, and that we worked with on a couple of recent fixes.

Seems to me there's room for both to be right. If PyJavaType only sorts by name, it is presumably only interested in that, as one might expect from a Python class. But maybe we're doing that bit in a way that doesn't keep its contract. I haven't looked properly yet.

Hmm ... @DavidPerezIngeniero : what version is this? I can't make the line numbers correspond exactly, but 2.7.1 is nearest.

[robertpatrick]

Interesting reading...

• https://forum.image.sc/t/bypassing-a-jdk1-8-bug-comparison-method-violates-its-general-contract-timsort/50044
• https://bugs.java.com/bugdatabase/view_bug.do?bug_id=7075600

[DavidPerezIngeniero]

Hmm ... @DavidPerezIngeniero : what version is this? I can't make the line numbers correspond exactly, but 2.7.1 is nearest.

It's jython-standalone 2.7.1b3, but the method comparator implementation hasn't changed in latest versions.

[DavidPerezIngeniero]

Interesting reading...

• https://forum.image.sc/t/bypassing-a-jdk1-8-bug-comparison-method-violates-its-general-contract-timsort/50044
• https://bugs.java.com/bugdatabase/view_bug.do?bug_id=7075600

I've been using Java 17, not Java 8.

[jeff5]

Good finds @robertpatrick . The bug report claims resolved in Java 9, so we should assume the general problem is fixed in @DavidPerezIngeniero's Java 17.

Tricky to investigate without a reliable way to reproduce.

[robertpatrick]

@jeff5 what would be an interesting data point would be for @DavidPerezIngeniero to try using the system property to disable TimSort to see if the issue still reproduces.