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

Comparison method violates its general contract! #349

Closed
yosvanyllr opened this issue Dec 22, 2023 · 18 comments · Fixed by #352
Closed

Comparison method violates its general contract! #349

yosvanyllr opened this issue Dec 22, 2023 · 18 comments · Fixed by #352
Assignees
Milestone

Comments

@yosvanyllr
Copy link

Hi,
WildFly 30 includes version 3.1.5
There on startup we can see a bug like #289

Caused by: org.jboss.as.server.deployment.DeploymentUnitProcessingException: WFLYSRV0156: Failed to index deployment root for annotations
at org.jboss.as.server@22.0.1.Final//org.jboss.as.server.deployment.annotation.ResourceRootIndexer.indexResourceRoot(ResourceRootIndexer.java:93)
at org.jboss.as.server@22.0.1.Final//org.jboss.as.server.deployment.annotation.AnnotationIndexProcessor.deploy(AnnotationIndexProcessor.java:34)
at org.jboss.as.server@22.0.1.Final//org.jboss.as.server.deployment.DeploymentUnitPhaseService.start(DeploymentUnitPhaseService.java:171)
... 8 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:1307)
at java.base/java.util.ArrayList.sort(ArrayList.java:1721)
at io.smallrye.jandex//org.jboss.jandex.Indexer.propagateTypeParameterBounds(Indexer.java:2570)
at io.smallrye.jandex//org.jboss.jandex.Indexer.complete(Indexer.java:2550)
at org.jboss.as.server@22.0.1.Final//org.jboss.as.server.deployment.annotation.ResourceRootIndexer.indexResourceRoot(ResourceRootIndexer.java:89)
... 10 more

BR

@Ladicek
Copy link
Collaborator

Ladicek commented Jan 2, 2024

That is weird, Jandex 3.1.5 should include the fix. Can you by any chance share the deployment that causes the issue?

@yosvanyllr
Copy link
Author

Hi,
As far I can debugging in to the lib, it is a comparative thing but not exactly the same error.
There are some limitations so I can't include here the deployment, Sorry.
Do you have some example to compare with.
Maybe the maven ear/war plugin is adding some non expected content.
Br

@Ladicek
Copy link
Collaborator

Ladicek commented Jan 2, 2024

Understood. I will look at the comparator implementation again, perhaps I will spot a mistake, but I'm not holding my breath.

If you have some time, it should be possible to trim down your deployment to a minimal reproducer like this:

  1. Find the JAR that makes Jandex fail.
  2. Use the Jandex JAR itself to build an index: java -jar jandex-3.1.5.jar .../path/to/failing.jar (this should fail for the first time)
  3. Remove a bunch of random .class files from the JAR and try indexing (per step 2) again. Pay extra attention to nested classes, as those are most likely to cause the problem. Keep doing that until indexing succeeds. Then put the class files removed in the last iteration back, remove some other ones, and repeat.

It should be possible to trim down the JAR to a small number of files that cause the problem. Then, you should be able to recreate a small JAR with mostly empty classes that exhibit the problem.

If you could do that, I'd be grateful, but I understand you most likely don't have the time for that.

@yosvanyllr
Copy link
Author

Hi,
Here are the results.
All indicates that manually there are no error on indexing.
Maybe is related to the way Wildfly is using the lib.

#:~/Downloads> java --version
openjdk 17.0.9 2023-10-17
OpenJDK Runtime Environment
OpenJDK 64-Bit Server VM (build ###-###, mixed mode, sharing)

#:~/Downloads> java -jar ~/install/wildfly-30.0.0.Final/modules/system/layers/base/io/smallrye/jandex/main/jandex-3.1.5.jar myear.ear
Wrote /home/base/myear-ear.idx in 0.0300 seconds (0 classes, 0 annotations, 0 instances, 0 class usages, 21 bytes)

#:~/Downloads> java -jar ~/install/wildfly-30.0.0.Final/modules/system/layers/base/io/smallrye/jandex/main/jandex-3.1.5.jar interface.war
Wrote /home/base/interface-war.idx in 0.0410 seconds (1 classes, 1 annotations, 2 instances, 28 class usages, 638 bytes)

#:~/Downloads> java -jar ~/install/wildfly-30.0.0.Final/modules/system/layers/base/io/smallrye/jandex/main/jandex-3.1.5.jar services.war
Wrote /home/base/services-war.idx in 0.2600 seconds (451 classes, 44 annotations, 2653 instances, 5424 class usages, 245047 bytes)

Br

@Ladicek
Copy link
Collaborator

Ladicek commented Jan 2, 2024

I don't think that java -jar jandex.jar my.war (or my.ear) will reproduce the problem, because that will only index the .class files present in the WAR (or EAR), but will ignore nested JARs. You first need to find the JAR that exhibits the problem.

@yosvanyllr
Copy link
Author

Hi, Thank's
@Ladicek
Issue found.
It was a dependency not compliant with latest java. I had to configure it to run as legacy:
-Djava.util.Arrays.useLegacyMergeSort=true
Br

@Ladicek
Copy link
Collaborator

Ladicek commented Jan 2, 2024

Is that dependency an opensource library? I would still like to find out the cause and fix the bug (because it is a Jandex bug).

@yosvanyllr
Copy link
Author

Hi,
Unfortunately it's a proprietary one.
We are opening a bug for them to review this issue.
Br

@Ladicek
Copy link
Collaborator

Ladicek commented Jan 2, 2024

OK, understood. Please note that this is truly a Jandex bug, that dependency cannot (and should not) do anything to fix this problem.

Now that you know which JAR exhibits the issue, you could possibly apply the "trial and error" method I described above to create a minimal failing JAR. I assume such minimal failing JAR will likely contain not more than 5 classes (based on my previous experiments with minimizing JARs that demonstrate Jandex bugs).

Then, a reproducer could be created with mostly empty classes. What matters is class names, class nesting and type variables, everything else should be irrelevant. If you (or the other team, or perhaps a 3rd party vendor?) can do that, that would be great!

@yosvanyllr
Copy link
Author

Hi,
The mentioned dependency is https://www.jxcell.net/
Br

@Ladicek
Copy link
Collaborator

Ladicek commented Jan 2, 2024

Interesting, I tried to download the ZIP from their website and index the jxcell.jar, but it worked just fine, no error. Even Jandex 3.0.0 (which has the bug) was able to index the JAR. Maybe there's a difference between their trial version and what you actually have 🤷

@yosvanyllr
Copy link
Author

Yes we also notice that.
The version we buy (as per the jxcell team) include this bug.
The latest seams not to have this problem.
BR.

@Ladicek
Copy link
Collaborator

Ladicek commented Jan 12, 2024

Hi @yosvanyllr, I have a Jandex branch here https://github.com/Ladicek/jandex/commits/index-postprocessing-sort/ that tries to avoid the problem altogether by sorting classes in a completely different way. I'm running tests myself, but if you could build Jandex from that branch, replace the Jandex JAR in your WildFly with that build and test, that would help. Thank you!

@yosvanyllr
Copy link
Author

Hi @Ladicek
Here I'm sharing with you the problematic lib jar
jxcell.gz

@Ladicek
Copy link
Collaborator

Ladicek commented Jan 13, 2024

Thanks!

@Ladicek
Copy link
Collaborator

Ladicek commented Feb 12, 2024

Hi, sorry for not getting to this sooner, I was busy with a few other things. Also, it took me a while to actually understand the problem -- the suggestions in my previous comments here were rather useless in this particular case. I eventually had to resort to a brute force approach and write a checker that verifies whether given Comparator indeed establishes a total order on a given set of values, using the actual definition of a total order. This has immediately shown where the problem is.

It also turns out that the experiment with a different comparator was actually a good idea, because that implementation in fact does provide a total order.

Fix is in #352.

@Ladicek
Copy link
Collaborator

Ladicek commented Mar 15, 2024

Hi @yosvanyllr, this should be fixed now in Jandex 3.1.7. I don't know when WildFly picks it up, but you should be able to replace any Jandex 3.1 release with 3.1.7 if you want to verify that the fix is effective.

@yosvanyllr
Copy link
Author

Thank you @Ladicek for the update.
We are updating our intallation by downloading this new version.
BR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants