Bug - Fixed Comparison method exception in textui.parsetree.SortBy (Java 7)

C

Catch-22

Active member
The new sorting method from java.util.Arrays.sort in Java 7 introduces an IllegalArgumentException when it detects a comparable that violates the comparable contract.

Java SE 7 and JDK 7 Compatibility
Area: API: Utilities
Synopsis: Updated sort behavior for Arrays and Collections may throw an IllegalArgumentException
Description: The sorting algorithm used by java.util.Arrays.sort and (indirectly) by java.util.Collections.sort has been replaced. The new sort implementation may throw an IllegalArgumentException if it detects a Comparable that violates the Comparable contract. The previous implementation silently ignored such a situation.
If the previous behavior is desired, you can use the new system property, java.util.Arrays.useLegacyMergeSort, to restore previous mergesort behavior.
Nature of Incompatibility: behavioral
RFE: 6804124

KoLmafia throws this exception, so I guess the comparables will need to be checked prior to sorting or something.
 
Veracity

Veracity

Developer
Staff member
Catch-22 said:
KoLmafia throws this exception
Click to expand...
Sure would be nice to know which Comparable objects we are sorting that have inappropriate compareTo methods. We could easily deduce that from the stack trace.

Speaking of which, where are your stack traces?
 
roippi

roippi

Developer
If you're referring to the previous fortune cookie stack trace where this exception popped up, the "contract violation" arose from two threads interfering with one another. There was no issue with the elements not properly implementing Comparable.
 
C

Catch-22

Active member
Hmm, is there any easy way for me to print out a stack trace? I tried using backtrace but that just seems to print a debug log.
 
C

Catch-22

Active member
*shrug* I thought Veracity was asking for something a little more helpful. Here's the debug log, I don't think it has the information Veracity is looking for though.
 

Attachments

  • DEBUG_20120705.txt
    4.2 KB · Views: 42
Veracity

Veracity

Developer
Staff member
If it takes an exception, we trap it and generate a stack trace in the debug log. That's exactly what I asked for. Here is your stack trace. From it, I can deduce some things. In particular, it was executing a consult script in a fight. Your consult script was executing an ASH "sort". And, since this is an unexpected runtime error, we did not trap it and print the file and line number where it occurred.

I'll look into making ASH "sort" trap runtime exceptions and print something more meaningful, much as we do for array index out of bounds and various library functions.

What consult script are you using?

Code: 
Unexpected error, debug log printed.
class java.lang.IllegalArgumentException: Comparison method violates its general contract!
java.lang.IllegalArgumentException: Comparison method violates its general contract!
	at java.util.ComparableTimSort.mergeLo(Unknown Source)
	at java.util.ComparableTimSort.mergeAt(Unknown Source)
	at java.util.ComparableTimSort.mergeCollapse(Unknown Source)
	at java.util.ComparableTimSort.sort(Unknown Source)
	at java.util.ComparableTimSort.sort(Unknown Source)
	at java.util.Arrays.sort(Unknown Source)
	at net.sourceforge.kolmafia.textui.parsetree.SortBy.execute(SortBy.java:112)
	at net.sourceforge.kolmafia.textui.parsetree.BasicScope.execute(BasicScope.java:465)
	at net.sourceforge.kolmafia.textui.parsetree.UserDefinedFunction.execute(UserDefinedFunction.java:139)
	at net.sourceforge.kolmafia.textui.parsetree.FunctionCall.execute(FunctionCall.java:169)
	at net.sourceforge.kolmafia.textui.parsetree.BasicScope.execute(BasicScope.java:465)
	at net.sourceforge.kolmafia.textui.parsetree.UserDefinedFunction.execute(UserDefinedFunction.java:139)
	at net.sourceforge.kolmafia.textui.parsetree.FunctionCall.execute(FunctionCall.java:169)
	at net.sourceforge.kolmafia.textui.parsetree.BasicScope.execute(BasicScope.java:465)
	at net.sourceforge.kolmafia.textui.parsetree.Loop.execute(Loop.java:59)
	at net.sourceforge.kolmafia.textui.parsetree.WhileLoop.execute(WhileLoop.java:100)
	at net.sourceforge.kolmafia.textui.parsetree.BasicScope.execute(BasicScope.java:465)
	at net.sourceforge.kolmafia.textui.parsetree.Else.execute(Else.java:65)
	at net.sourceforge.kolmafia.textui.parsetree.If.execute(If.java:82)
	at net.sourceforge.kolmafia.textui.parsetree.BasicScope.execute(BasicScope.java:465)
	at net.sourceforge.kolmafia.textui.parsetree.Conditional.execute(Conditional.java:96)
	at net.sourceforge.kolmafia.textui.parsetree.If.execute(If.java:68)
	at net.sourceforge.kolmafia.textui.parsetree.BasicScope.execute(BasicScope.java:465)
	at net.sourceforge.kolmafia.textui.parsetree.UserDefinedFunction.execute(UserDefinedFunction.java:139)
	at net.sourceforge.kolmafia.textui.parsetree.FunctionCall.execute(FunctionCall.java:169)
	at net.sourceforge.kolmafia.textui.parsetree.Assignment.execute(Assignment.java:103)
	at net.sourceforge.kolmafia.textui.parsetree.BasicScope.execute(BasicScope.java:465)
	at net.sourceforge.kolmafia.textui.parsetree.UserDefinedFunction.execute(UserDefinedFunction.java:139)
	at net.sourceforge.kolmafia.textui.parsetree.FunctionCall.execute(FunctionCall.java:169)
	at net.sourceforge.kolmafia.textui.parsetree.Assignment.execute(Assignment.java:103)
	at net.sourceforge.kolmafia.textui.parsetree.BasicScope.execute(BasicScope.java:465)
	at net.sourceforge.kolmafia.textui.parsetree.Loop.execute(Loop.java:59)
	at net.sourceforge.kolmafia.textui.parsetree.RepeatUntilLoop.execute(RepeatUntilLoop.java:78)
	at net.sourceforge.kolmafia.textui.parsetree.BasicScope.execute(BasicScope.java:465)
	at net.sourceforge.kolmafia.textui.parsetree.UserDefinedFunction.execute(UserDefinedFunction.java:139)
	at net.sourceforge.kolmafia.textui.Interpreter.executeScope(Interpreter.java:369)
	at net.sourceforge.kolmafia.textui.Interpreter.execute(Interpreter.java:296)
	at net.sourceforge.kolmafia.textui.Interpreter.execute(Interpreter.java:289)
	at net.sourceforge.kolmafia.request.FightRequest.nextRound(FightRequest.java:694)
	at net.sourceforge.kolmafia.request.FightRequest.runOnce(FightRequest.java:1349)
	at net.sourceforge.kolmafia.request.FightRequest.run(FightRequest.java:1375)
	at net.sourceforge.kolmafia.webui.RelayAutoCombatThread.run(RelayAutoCombatThread.java:74)
 
Veracity

Veracity

Developer
Staff member
Revision 11245 traps that exception and prints filename and line number. When it happens again, we can examine your script.
 
C

Catch-22

Active member
Veracity said:
Revision 11245 traps that exception and prints filename and line number. When it happens again, we can examine your script.
Click to expand...

Oh, I was planning on telling you that when I got back to my regular PC. It's zarqon's BatBrain.ash, line 979.

Edit: I understand how much of a beast BatBrain is, so I was actually planning on just writing a code snippet that causes this exception. Feel free to just take a look at BatBrain though, I am going to be fairly busy for the next few hours.
 
Last edited:
fronobulax

fronobulax

Developer
Staff member
For the record, one of the lint tools has complained about some of the comparable implementations in KoLmafia. This computer does not have the tools but I seem to recall compare functions that assume, rather than check, a type, hash functions that are not implemented and a couple other things. While a Java 1.7 problem is out of scope today, if these are detectable now I'll pay attention and start cleaning them up.
 
C

Catch-22

Active member
Tracked this sucker down. The problem stems from Expression.evalInternal(). As frono hinted at, we're assuming this function will always return a float. Here's a small snippet to show that this isn't always the case.
Code: 
> ash modifier_eval("sqrt(-1)")

Returned: �

According to the Java API for math.sqrt "If the argument is NaN or less than zero, then the result is NaN" and I'm guessing Java replaces NaN with Character.toString((char) 65533), the unicode replacement character, somewhere along the line.

Because of the accuracy lost in all the floating point operations used by BatBrain, it will occasionally end up with something like "-0.000003051757815342171" instead of "0", which leads to buggy behaviour when BatBrain does some of it's eval expressions.

I guess for square root of negative numbers, eval() could just return 0.0f.

There's a few different ways you could go about fixing this one, so I'll leave it up to the devs to provide a patch.
 
Last edited:
fronobulax

fronobulax

Developer
Staff member
r11259 maps ash modifier_eval("sqrt(-1)") into 0.0, along with anything else where Math.sqrt returns NaN. I'm not really sure it is worth detecting a negative argument to sqrt and trying to inform the user but if someone else does this is easily reverted.
 
Veracity

Veracity

Developer
Staff member
I would prefer that it threw a runtime exception - as we do when you try to divide by zero - so people can correct their scripts.
 
C

Catch-22

Active member
Veracity said:
Making the sqrt( -1 ) return 0 is simply not math.
Click to expand...

I totally agree with you on this one Veracity. The reason I left this up to the devs to make the call was because throwing an exception will (at least in the short term) likely break BatBrain and by extension the widely used SmartStasis/WHAM scripts.

I guess what I'm trying to say is, it would've been nice if this exception was being handled before thousands of players starting using a script that will be affected by the change, but that's life I suppose.
 
Last edited:
jasonharper

jasonharper

Developer
r11264 fixes the ACTUAL problem here, which is that Value.compareTo() was violating the requirements for comparator methods.
 
You must log in or register to reply here.
Top