-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Comments
I don't think there is any race. The exception happens in a JDK method, namely I'm closing as I don't think it's a Jetty issue. |
@andresluuk said in #10059: I belived before there is a jetty bug in XmlConfiguration. 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 |
Fixed the executable comparitor to always be transitive.
@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. |
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? |
I've pushed an update to filter before sorting |
Fixed the executable comparitor to always be transitive.
Fix #10143 executable comparator Signed-off-by: gregw <[email protected]>
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]>
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
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.
The text was updated successfully, but these errors were encountered: