Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Startup fails due to IllegalArgumentException: Comparison method violates its general contract #10143

Closed
andresluuk opened this issue Jul 25, 2023 · 5 comments · Fixed by #10156
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@andresluuk
Copy link

andresluuk commented Jul 25, 2023

Jetty version(s)
I have seen it 2 times out of a 1000 runs or so, once on jetty12ee9 beta3 and once on jetty12ee8 built today.
Java version/vendor (use: java -version)
JDK17
OS type/version
Unix
Description
Sometimes at startup server configuration throws an error. I think it might be some race condition? (It might also be caused by jrebel somehow).
Config error java.lang.IllegalArgumentException: Comparison method violates its general contract! at | | in file:///opt/containers/jetty-home-12.0.0.beta3/etc/jetty-ee8-webapp.xml

I will attach the console log file.

jetty_console.log

2023-07-25 09:54:43.415:WARN :oejx.XmlConfiguration:main: Config error java.lang.IllegalArgumentException: Comparison method violates its general contract! at <Call class="org.eclipse.jetty.ee8.webapp.WebAppContext" name="addSystemClasses"><Arg><Ref refid="Environment"/></Arg><Arg>|      <Call class="org.eclipse.jetty.util.StringUtil" name="csvSplit"><Arg><Property name="jetty.webapp.addSystemClasses"/></Arg></Call>|    </Arg></Call> in file:///opt/containers/jetty-home-12.0.0.beta3/etc/jetty-ee8-webapp.xml
2023-07-25 09:54:43.420:WARN :oejx.XmlConfiguration:main: Unable to execute XmlConfiguration
java.lang.RuntimeException: java.lang.IllegalArgumentException: Comparison method violates its general contract!
	at org.eclipse.jetty.xml.XmlConfiguration.lambda$main$2(XmlConfiguration.java:1957)
	at org.eclipse.jetty.util.component.Environment.run(Environment.java:75)
	at org.eclipse.jetty.xml.XmlConfiguration.main(XmlConfiguration.java:1964)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.eclipse.jetty.start.Main.invokeMain(Main.java:221)
	at org.eclipse.jetty.start.Main.start(Main.java:519)
	at org.eclipse.jetty.start.Main.main(Main.java:76)
Caused by: 
java.lang.IllegalArgumentException: Comparison method violates its general contract!
	at java.base/java.util.TimSort.mergeLo(TimSort.java:781)
	at java.base/java.util.TimSort.mergeAt(TimSort.java:518)
	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.eclipse.jetty.xml.XmlConfiguration$JettyXmlConfiguration.call(XmlConfiguration.java:954)
	at org.eclipse.jetty.xml.XmlConfiguration$JettyXmlConfiguration.call(XmlConfiguration.java:932)
	at org.eclipse.jetty.xml.XmlConfiguration$JettyXmlConfiguration.configure(XmlConfiguration.java:490)
	at org.eclipse.jetty.xml.XmlConfiguration$JettyXmlConfiguration.configure(XmlConfiguration.java:443)
	at org.eclipse.jetty.xml.XmlConfiguration.configure(XmlConfiguration.java:345)
	at org.eclipse.jetty.xml.XmlConfiguration.lambda$main$2(XmlConfiguration.java:1950)
	at org.eclipse.jetty.util.component.Environment.run(Environment.java:75)
	at org.eclipse.jetty.xml.XmlConfiguration.main(XmlConfiguration.java:1964)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.eclipse.jetty.start.Main.invokeMain(Main.java:221)
	at org.eclipse.jetty.start.Main.start(Main.java:519)
	at org.eclipse.jetty.start.Main.main(Main.java:76)
java.lang.reflect.InvocationTargetException
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.eclipse.jetty.start.Main.invokeMain(Main.java:221)
	at org.eclipse.jetty.start.Main.start(Main.java:519)
	at org.eclipse.jetty.start.Main.main(Main.java:76)
Caused by: java.lang.RuntimeException: java.lang.IllegalArgumentException: Comparison method violates its general contract!
	at org.eclipse.jetty.xml.XmlConfiguration.lambda$main$2(XmlConfiguration.java:1957)
	at org.eclipse.jetty.util.component.Environment.run(Environment.java:75)
	at org.eclipse.jetty.xml.XmlConfiguration.main(XmlConfiguration.java:1964)
	... 7 more
Caused by: java.lang.IllegalArgumentException: Comparison method violates its general contract!
	at java.base/java.util.TimSort.mergeLo(TimSort.java:781)
	at java.base/java.util.TimSort.mergeAt(TimSort.java:518)
	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.eclipse.jetty.xml.XmlConfiguration$JettyXmlConfiguration.call(XmlConfiguration.java:954)
	at org.eclipse.jetty.xml.XmlConfiguration$JettyXmlConfiguration.call(XmlConfiguration.java:932)
	at org.eclipse.jetty.xml.XmlConfiguration$JettyXmlConfiguration.configure(XmlConfiguration.java:490)
	at org.eclipse.jetty.xml.XmlConfiguration$JettyXmlConfiguration.configure(XmlConfiguration.java:443)
	at org.eclipse.jetty.xml.XmlConfiguration.configure(XmlConfiguration.java:345)
	at org.eclipse.jetty.xml.XmlConfiguration.lambda$main$2(XmlConfiguration.java:1950)
	... 9 more
java.lang.reflect.InvocationTargetException
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.eclipse.jetty.start.Main.invokeMain(Main.java:221)
	at org.eclipse.jetty.start.Main.start(Main.java:519)
	at org.eclipse.jetty.start.Main.main(Main.java:76)
Caused by: java.lang.RuntimeException: java.lang.IllegalArgumentException: Comparison method violates its general contract!
	at org.eclipse.jetty.xml.XmlConfiguration.lambda$main$2(XmlConfiguration.java:1957)
	at org.eclipse.jetty.util.component.Environment.run(Environment.java:75)
	at org.eclipse.jetty.xml.XmlConfiguration.main(XmlConfiguration.java:1964)
	... 7 more
Caused by: java.lang.IllegalArgumentException: Comparison method violates its general contract!
	at java.base/java.util.TimSort.mergeLo(TimSort.java:781)
	at java.base/java.util.TimSort.mergeAt(TimSort.java:518)
	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.eclipse.jetty.xml.XmlConfiguration$JettyXmlConfiguration.call(XmlConfiguration.java:954)
	at org.eclipse.jetty.xml.XmlConfiguration$JettyXmlConfiguration.call(XmlConfiguration.java:932)
	at org.eclipse.jetty.xml.XmlConfiguration$JettyXmlConfiguration.configure(XmlConfiguration.java:490)
	at org.eclipse.jetty.xml.XmlConfiguration$JettyXmlConfiguration.configure(XmlConfiguration.java:443)
	at org.eclipse.jetty.xml.XmlConfiguration.configure(XmlConfiguration.java:345)
	at org.eclipse.jetty.xml.XmlConfiguration.lambda$main$2(XmlConfiguration.java:1950)
	... 9 more

Usage: java -jar $JETTY_HOME/start.jar [options] [properties] [configs]
       java -jar $JETTY_HOME/start.jar --help  # for more information

We start up jetty with a simple start.ini in the profile and it generates all other conf based on that.
If you feel like there is no race there, then you can probably close the case and it might be an issue caused by jrebels presence. But jrebels presence might make things slower and this can expose race conditions.

@andresluuk andresluuk added the Bug For general bugs on Jetty side label Jul 25, 2023
@sbordet
Copy link
Contributor

sbordet commented Jul 25, 2023

I don't think there is any race.

The exception happens in a JDK method, namely TimSort.mergeLo(TimSort.java:781), which I guess it's a JRebel glitch.
I would report this to JRebel.

I'm closing as I don't think it's a Jetty issue.

@gregw
Copy link
Contributor

gregw commented Jul 26, 2023

@andresluuk said in #10059:

I belived before there is a jetty bug in XmlConfiguration.
So what happens:
Arrays.sort(someClass.getMethods(), EXECUTABLE_COMPARATOR);
The method order returned is random and the error happens only if the methods are in a bad order in the original array.
What is the cause of the error?
EXECUTABLE_COMPARATOR is a Comparator and the compare method has the following requirement:
The implementor must also ensure that the relation is transitive: ((compare(x, y)>0) && (compare(y, z)>0)) implies compare(x, z)>0
Looking at the code of EXECUTABLE_COMPARATOR I see that this check will fail in the comparator in this class.
Anyway you must be a little unlucky to hit the error from the JDK internal sorter. I also made a sample that shows that the transitivity is broken:

import java.lang.reflect.Executable;
import java.lang.reflect.Method;
import java.lang.reflect.Parameter;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Comparator;
import java.util.List;
import java.util.concurrent.Callable;

public class Test {

  public static final Comparator<Executable> EXECUTABLE_COMPARATOR = (e1, e2) -> {
    // Favour methods with less parameters
    int count = e1.getParameterCount();
    int compare = Integer.compare(count, e2.getParameterCount());
    if (compare == 0 && count > 0) {
      Parameter[] p1 = e1.getParameters();
      Parameter[] p2 = e2.getParameters();

      // Favour methods without varargs
      compare = Boolean.compare(p1[count - 1].isVarArgs(), p2[count - 1].isVarArgs());
      if (compare == 0) {
        // Rank by differences in the parameters
        for (int i = 0; i < count; i++) {
          Class<?> t1 = p1[i].getType();
          Class<?> t2 = p2[i].getType();
          if (t1 != t2) {
            // Favour derived type over base type
            compare = Boolean.compare(t1.isAssignableFrom(t2), t2.isAssignableFrom(t1));
            if (compare == 0) {
              // favour primitive type over reference
              compare = Boolean.compare(!t1.isPrimitive(), !t2.isPrimitive());
              if (compare == 0)
                // Use name to avoid non determinant sorting
                compare = t1.getName().compareTo(t2.getName());
            }

            // break on the first different parameter (should always be true)
            if (compare != 0)
              break;
          }
        }
      }
      compare = Math.min(1, Math.max(compare, -1));
    }
//    System.out.println(e1.getName() + " vs " + e2.getName() + " " + compare + " " + count + " " + e2.getParameterCount());
    return compare;
  };

  public static void test() throws Exception {
    main(null);
  }

  public static void main(String args) throws Exception {
    System.out.println("start sort");
    Method[] methodsorig = Test.class.getMethods();
    Arrays.sort(methodsorig, EXECUTABLE_COMPARATOR);
    List<Method> filtered = new ArrayList<Method>();
    for (Method m : methodsorig) {
      if ("yy".equals(m.getName()) || "bb".equals(m.getName()) || "zz".equals(m.getName())) {
        filtered.add(m);
      }
    }
    Method[] methods = filtered.toArray(new Method[filtered.size()]);
    Arrays.sort(methods, EXECUTABLE_COMPARATOR);
    int[][] b = new int[methods.length][methods.length];
    for (int x = 0; x < methods.length; x++) {
      for (int y = 0; y < methods.length; y++) {
        b[x][y] = EXECUTABLE_COMPARATOR.compare(methods[x], methods[y]);
      }
    }
    System.out.println("start sort done for " + methods.length);
    for (int x = 0; x < methods.length; x++) {
      for (int y = 0; y < methods.length; y++) {
        System.out.print(b[x][y] + " ");
      }
      System.out.println(" " + methods[x].getName() + " " + x);
    }
//    The implementor must also ensure that the relation is transitive: ((compare(x, y)>0) && (compare(y, z)>0)) implies compare(x, z)>0
    test(methods[2], methods[1]);
    test(methods[1], methods[0]);
    test(methods[2], methods[0]);
    Thread.sleep(500);
  }

  public static void test(Method x, Method y) throws Exception {
    System.out.println(x.getName() + " vs " + y.getName() + " = " + EXECUTABLE_COMPARATOR.compare(x, y));
  }

  public void bb(BBB i) throws Exception {
  }

  public void yy(YYY i) throws Exception {
  }

  public void zz(ZZZ i) throws Exception {
  }

  public static interface BBB {
  }

  public static class YYY {
  }

  public static class ZZZ implements BBB {
  }
}

The issue is in this line, which breaks if some classes are not in hierarchy
compare = Boolean.compare(t1.isAssignableFrom(t2), t2.isAssignableFrom(t1));
And a post about: Comparison method violates its general contract!
https://stackoverflow.com/questions/11441666/java-error-comparison-method-violates-its-general-contract

@gregw gregw reopened this Jul 26, 2023
@gregw gregw self-assigned this Jul 26, 2023
gregw added a commit that referenced this issue Jul 26, 2023
Fixed the executable comparitor to always be transitive.
@gregw
Copy link
Contributor

gregw commented Jul 26, 2023

@andresluuk wow, you must be unlucky to hit a bug that we have NEVER seen, but has existed from a long long time. Thanks for the report and the persistence!

See #10152 for an attempt to fix it.

@andresluuk
Copy link
Author

JRebel can add some methods to the class, so it probably made the issue more likely! Maybe jetty 12 added more configuration stuff then there was on jetty11 so it was less likely before. Or there is new class passed through the sort that is more complex then older ones?
But I checked the commit and it seems that it would fix the sorting issue.
I think that you could also filter the list by method name before sorting it, but as this code is not executed much then this will not matter much.

@joakime joakime linked a pull request Jul 26, 2023 that will close this issue
gregw added a commit that referenced this issue Jul 26, 2023
@gregw
Copy link
Contributor

gregw commented Jul 26, 2023

I've pushed an update to filter before sorting

gregw added a commit that referenced this issue Jul 26, 2023
Fixed the executable comparitor to always be transitive.
gregw added a commit that referenced this issue Jul 26, 2023
gregw added a commit that referenced this issue Jul 26, 2023
Fix #10143 executable comparator

Signed-off-by: gregw <[email protected]>
sbordet pushed a commit that referenced this issue Jul 26, 2023
Fixed the executable comparator to always be transitive.

Signed-off-by: gregw <[email protected]>
Signed-off-by: Joakim Erdfelt <[email protected]>
Co-authored-by: Joakim Erdfelt <[email protected]>
@joakime joakime changed the title Jetty12 startup fails at random? Startup fails due to IllegalArgumentException: Comparison method violates its general contract Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
None yet
3 participants