Sorting list of files in android throwing 'Comparison method violates its general contract!'

Question

It's happening on my Android application & here is the stack trace:

Caused by java.lang.IllegalArgumentException: Comparison method violates its general contract!
       at java.util.TimSort.mergeHi(TimSort.java:864)
       at java.util.TimSort.mergeAt(TimSort.java:481)
       at java.util.TimSort.mergeForceCollapse(TimSort.java:422)
       at java.util.TimSort.sort(TimSort.java:219)
       at java.util.TimSort.sort(TimSort.java:169)
       at java.util.Arrays.sort(Arrays.java:2023)
       at java.util.Collections.sort(Collections.java:1883)

Here is my sort logic:

private static void sortFiles(List<File> listFiles, int sortDirection) {
    try {
      if (sortDirection == sortLatestFirst) {
        Collections.sort(listFiles, new LatestFirstComparator());
        ...

Here is the comparator:

class LatestFirstComparator implements Comparator<File> {
  @Override
  public int compare(File f1, File f2) {
    return Long.compare(f2.lastModified(), f1.lastModified());
  }
}

I've found related questions & other solutions, but none of them solves my problem. Moreover, this is not a consistent behavior, but happening only with some of the app users.

Does `lastModified()` return a memory-stored value or a current file's modification date? In the latter case, _if_ the file is modified during the sort, the final order could change _during_ sorting, which would be detected as a comparator instability (for example the 'is newer' relation may appear non-transitive: A is detected as newer than B, and B newer than C, but then C is modified and appears newer than A). — CiaPan, Oct 16 '19 at 18:23
The `lastModified()` gives the file's modification date as given by the file system. But the sorting is happening on an ArrayList of files derived from `directory.listFiles()`. Will the underlying file modifications reflect in the ArrayList elements? — Adi, Oct 17 '19 at 17:09
Possibly. I'd expect the list contains (refers to) exactly same objects `directory` returned, not their copies. If you need stable data, I suppose you could make your own objects with copies of necessary attributes; then the ordering will be well-defined, but... the final order may appear obsolete if files are modified during sorting. Anyway you can't avoid that in a multitasking environment; you'll never know when some other thread or process creates, modifies or deletes files until you re-scan the directory (unless the other party notifies you, which I doubt). — CiaPan, Oct 18 '19 at 08:04
Apache commons contains [class `LastModifiedFileComparator`](https://commons.apache.org/proper/commons-io/javadocs/api-2.5/org/apache/commons/io/comparator/LastModifiedFileComparator.html) but I suspect it would suffer the same issue, too. The linked page does not give any guarantees... — CiaPan, Oct 18 '19 at 08:08

score 4 · Accepted Answer · answered Oct 25 '19 at 12:28

As said by others, the problem lies in the fact that the value of the last modified timestamp can change during the sort operation. In order to sort reliably, you have to cache the values for the duration of the sort operation:

private static void sortFiles(List<File> listFiles, int sortDirection) {
    if(listFiles.isEmpty()) return;
    Map<File,Long> cache = new HashMap<>();
    Comparator<File> byTime
        = Comparator.comparing(f -> cache.computeIfAbsent(f, File::lastModified));
    if(sortDirection == sortLatestFirst) byTime = byTime.reversed();
    listFiles.sort(byTime);
}

I assume that you use a notification mechanism for changes anyway, to decide when to reload the file list, so the sorting operation does not need to deal with it.

If you want to support an API level that doesn’t support Java 8 features, you have to use the more verbose variant

private static void sortFiles(List<File> listFiles, int sortDirection) {
    if(listFiles.isEmpty()) return;
    final Map<File,Long> cache = new HashMap<File,Long>();
    Comparator<File> byTime = new Comparator<File>() {
        @Override
        public int compare(File f1, File f2) {
            Long t1 = cache.get(f1), t2 = cache.get(f2);
            if(t1 == null) cache.put(f1, t1 = f1.lastModified());
            if(t2 == null) cache.put(f2, t2 = f2.lastModified());
            return t1.compareTo(t2);
        }
    };
    if(sortDirection == sortLatestFirst) byTime = Collections.reverseOrder(byTime);
    Collections.sort(listFiles, byTime);
}

I think I was meaning something like that, when I said in the comment _'If you need stable data, I suppose you could make your own objects with copies of necessary attributes.'_ But I'm not fluent in Java, so could not provide such elegant solution as you did, Upvoted. — CiaPan, Oct 25 '19 at 15:48
@CiaPan you should’ve said this in your answer as well. Comments are easily overlooked. — Holger, Oct 25 '19 at 15:50
Thank you for the advice, I think I'll follow it and copy main points from my comments to the answer. I wrote my answer as a kind of summary or rather a conclusion for the talk done previously in the comments space. I completely missed the fact the answer will be read later by other, uninvolved people, who may skip comments and miss the context. Thanks again for pointing this out. — CiaPan, Oct 25 '19 at 16:00

CiaPan · Answer 2 · 2019-10-25T20:13:47.563

EDIT: Following an advice from the comment by User Holger I'm expanding the answer by adding here important points from my previous comments. They are put in quotation blocks.

As I pointed out in comments, most probably some of the files being sorted are modified during the sorting. Then, what was initially an old file becomes a new file during sorting:

e.g. file A is detected as newer than B, and B newer than C, but then C is modified and appears newer than A

Thus the ordering relation 'is newer' appears non-transitive, which breaks the contract; hence the error.

If you need stable data, I suppose you could make your own objects with copies of necessary attributes; then the ordering will be well-defined, but... the final order may appear obsolete if files are modified during sorting. Anyway you can't avoid that in a multitasking environment; you'll never know when some other thread or process creates, modifies or deletes files until you re-scan the directory

IMHO you can only write your own sorting routine, which would handle such non-transitivity safely. For example it can reiterate when it detects the situation. But take care not to iterate too many times - if two or more files get updated constantly, the re-sorting routine could never finish its work!

Writing your own sorting routine is a big thing, especially when it should incorporate in-between changes. And, as you said, it’s prone to infinite update loops. There’s a simpler solution, just ensure to [use the first encountered timestamp rather than updated values](https://stackoverflow.com/a/58558560/2711488)… — Holger, Oct 25 '19 at 12:31

Sorting list of files in android throwing 'Comparison method violates its general contract!'

2 Answers2