Bug "Comparison method violates its general contract!"

fredg1

Member
Code_n45u2VRr9a.png
Code_Ja41bQG3IF.png

Not *entirely* sure of what is happening here, but I think nameArray is a cached array version of nameList (as it is not referenced anywhere else), but was not made thread-safe (there are currently multiple scripts being parsed at once, with maybe two at once accessing this method), so one thread is trying to sort the array while another is modifying it. Maybe.
 

fronobulax

Developer
Staff member
If you can tell me which class has a contract violating comparison method I will gladly clean my plate, track it down and write some tests. But "I think" and "maybe" are not strong enough to convince me that my time would be well spent. My experience with contract violations is that a threading issue unlikely to be a factor but in this case I will use the words indicating my uncertainty.
 

fredg1

Member
If you can tell me which class has a contract violating comparison method I will gladly clean my plate, track it down and write some tests.
I'm not sure what you're asking here. I'm already showing what class/method is the issue in the first image, in that it's net.sourceforge.kolmafia.persistence.AdventureDatabase.find(String), at line 851, which is a call to Arrays.sort(Object[]).

On top of the fact that no comparison method is submitted, the second screenshot shows that the object submitted (nameArray) is only referenced at those 5 locations inside this same method, which points to the cause being another thread.

The question/uncertainty wasn't whether or not there was a problem, it was what the cause was.
 

fredg1

Member
Additionally, to point further towards threading being the issue:
I ran the exact same test three times. The first time gave the error above. The second time didn't give an error, and the third gave this slightly different error (note the last two lines; this time, the exception was thrown from ComparableTimSort.mergeLo, but was from ComparableTimSort.mergeHi in the example above)
Code_kt9elJhHEs.png
This points towards the issue coming from outside interference.
 

Ryo_Sangnoir

Developer
Staff member
You could potentially eliminate `nameArray` entirely by making `nameList` a `SortedList`, and just doing `nameList.toArray(new String[0])` in the `getMatchingNames` call.

For test coverage, I would call from `AdventureDatabase.getAdventure` and confirm that `StringUtilities.getMatchingNames` is fully covered (as much as is reasonable). This avoids testing implementation details that may change in future refactors.
 
Top