B0rked in the tunnel builder: java.lang.IllegalArgumentException

[killyourtv]

I2P version: 0.9.3-15
Java version: Oracle Corporation 1.7.0_03 (OpenJDK Runtime Environment 1.7.0_03-b21)
Wrapper version: 3.5.16
Server version: 6.1.26
Servlet version: Jasper JSP 2.1 Engine
Platform: Linux amd64 3.2.0-4-amd64

A few minutes after starting, I had the following in my logs:

12/15/12 22:13:44.755 CRIT [uildExecutor] uter.tunnel.pool.BuildExecutor: B0rked in the tunnel builder
     java.lang.IllegalArgumentException: Comparison method violates its general contract!
     at java.util.TimSort.mergeHi(TimSort.java:868)
     at java.util.TimSort.mergeAt(TimSort.java:485)
     at java.util.TimSort.mergeCollapse(TimSort.java:410)
     at java.util.TimSort.sort(TimSort.java:214)
     at java.util.TimSort.sort(TimSort.java:173)
     at java.util.Arrays.sort(Arrays.java:659)
     at java.util.Collections.sort(Collections.java:217)
     at net.i2p.router.tunnel.pool.BuildExecutor.run(BuildExecutor.java:334)
     at java.lang.Thread.run(Thread.java:722)
     at net.i2p.util.I2PThread.run(I2PThread.java:85)

[zzz]

​http://stackoverflow.com/questions/7849539/comparison-method-violates-its-general-contract-java-7-only
​http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6804124
​http://www.oracle.com/technetwork/java/javase/compatibility-417013.html#source

[zab]

​https://en.wikipedia.org/wiki/Total_order

The BuildExecutor.TunnelPoolComparator looks fine unless the return values of getTunnelCount() or getSettings().isExploratory() can change while the list is being sorted. (I think? Can anyone find a case that breaks total order?)

Either set the property java.util.Arrays.useLegacyMergeSort=true to mute the exception (but risk that the wanted list may not always be sorted correctly) or apply ​http://pastethis.i2p/show/2488/

[zzz]

Agreed with your analysis. Only on Java 7. Minor because the BuildExecutor? will loop and try again.

Occasional mis-sorting in BuildExecutor? is not a big problem. However, Oracle claims a big speedup with TimSort? so I'd rather not set the property for the whole JVM.

Your paste 2488 may be overkill? My quick and dirty idea for a fix:

        try {
           Collections.sort(wanted, new TunnelPoolComparator());
        } catch (IllegalArgumentException iae) {
            wanted.clear();
            pools.clear();
            continue;
        }
}}}}    

[zab]

For now it should work. You're opening yourself up to future changes in the code inside the try clause that could throw an IAE that you do not want to ignore.

[zzz]

I neglected to answer your earlier question. isExploratory() will not change but getTunnelCount() may.

[zzz]

Patch similar to that in comment 3 above applied in 0.9.4-2.