JDK-8305072 : Win32ShellFolder2.compareTo is inconsistent
  • Type: Bug
  • Component: client-libs
  • Sub-Component: javax.swing
  • Affected Version: 17.0.8,19
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • OS: windows
  • Submitted: 2023-03-25
  • Updated: 2024-04-18
  • Resolved: 2024-04-09
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 23
23 b18Fixed
Related Reports
Relates :  
Description
ADDITIONAL SYSTEM INFORMATION :
Windows 11
Java version 19.0.2

A DESCRIPTION OF THE PROBLEM :
One of our Windows users gets an IllegalArgumentException from Arrays.sort() when trying to open a JFileChooser. For more details about the investigation we did, see here:

https://forum.vassalengine.org/t/cannot-open-window-for-load-save-or-beginlogs/76730/1

The cause of the problem is that Win32ShellFolder2.compareTo() and equals() disagree about how to order special and non-special folders.

Our user has _two_ instances of "C:\Users\<FOLDER>\Documents" in the array returned by Win32ShellFolderManager2.get(), one of which has isSpecial() true and the other false. Call the special one "a", the non-special one "b", and let "c" be a non-special Win32ShellFolder2 for "C:\Users\\<FOLDER>". We see the following from a test we had the user run:

  a.equals(b) == true

  a.compareTo(c) == -1
  b.compareTo(c) == 10
  a.compareTo(b) == 0

This means that a < c, c < b, and a = b, which means that the < relation induced by compareTo() is not a linear order, in violation of the requirements of the Comparable interface. This is why Arrays.sort() throws an exception when attempting to sort an array containing Win32ShellFolder2's like these.

The intention of Win32ShellFolder2.compareTo() is clearly to sort specials before non-specials---which is fine---but in that case equals() must actually check whether the two items' isSpecial() agtree, and it does not. The lack of this check is also what makes Win32ShellFolder2.compareTo() fail. Win32ShellFolder2.compareTo() calls Win32ShellFolderManager2.compareShellFolders(). In the case where at least one of the folders being compared is special, they are both checked against the topFolderList for their index---and ArrayList.indexOf() uses equals(). Hence, if a (which is special) equals() one of the items in the topFolderList, so will b, even though it should not because b is not special. In this case a and b have the same index in the list, and compareShellFolders() returns indexOf(a) - indexOf(b), which is 0, thus declaring a and b to be equal---when in fact it should return negative because a is special and b is not.

How can this be fixed?

* It's not clear to me whether anything relies on Win32ShellFolder2.equals() ignoring isSpecial(). If nothing does, then there is a very simple solution. Add 

  if (isSpecial() != rhs.isSpecial()) {
    return false;
  }

to Win32ShellFolder2.equals().

* If, on the other hand, something does depend on Win32ShellFolder2.equals() ignoring isSpecial(), the problem can be solved by adjusting Win32ShellFolderManager2.compareShellFolders():

The line

  if (special1 || special2) {

ought to be 

  if (special1 && special2) {

instead.

(In fact, that change should be made regardless---there is no reason to enter the block to check index in the topFolderList unless _both_ folders are special. If only one is special, the block after this one will handle it.)


STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
Run the test case on a Windows system where there is a special Win32ShellFolder2 in the topFolderList having the same path as a non-special Win32ShellFolder2, and also enough Win32ShellFolder2 in total which are children of `getDesktop()` to cause the "large" case of TimSort to execute.

I don't know how to trigger the condition where you have a duplicate non-specialof a special in the topFolderList, only that the problem is 100% reproducible once you do have this condition. We have been seeing user reports of this problem a few times a year since TimSort became the default in Java 7, but only just now were we able to work though the problem with a user who was willing to run numerous diagnostic cases we provided.

EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
File dialog opens with no exception.
ACTUAL -
java.lang.IllegalArgumentException: Comparison method violates its general contract! at java.base/java.util.ComparableTimSort.mergeHi(ComparableTimSort.java:870) at java.base/java.util.ComparableTimSort.mergeAt(ComparableTimSort.java:487) at java.base/java.util.ComparableTimSort.mergeForceCollapse(ComparableTimSort.java:426) at java.base/java.util.ComparableTimSort.sort(ComparableTimSort.java:222) at java.base/java.util.Arrays.sort(Arrays.java:1054) at java.desktop/sun.awt.shell.Win32ShellFolderManager2.get(Win32ShellFolderManager2.java:315) at java.desktop/sun.awt.shell.ShellFolder.get(ShellFolder.java:274) at java.desktop/com.sun.java.swing.plaf.windows.WindowsFileChooserUI$DirectoryComboBoxModel.addItem(WindowsFileChooserUI.java:1123) at java.desktop/com.sun.java.swing.plaf.windows.WindowsFileChooserUI.doDirectoryChanged(WindowsFileChooserUI.java:777) at java.desktop/com.sun.java.swing.plaf.windows.WindowsFileChooserUI$11.propertyChange(WindowsFileChooserUI.java:868) at java.desktop/java.beans.PropertyChangeSupport.fire(PropertyChangeSupport.java:343) at java.desktop/java.beans.PropertyChangeSupport.firePropertyChange(PropertyChangeSupport.java:335) at java.desktop/java.beans.PropertyChangeSupport.firePropertyChange(PropertyChangeSupport.java:268) at java.desktop/java.awt.Component.firePropertyChange(Component.java:8716) at java.desktop/javax.swing.JFileChooser.setCurrentDirectory(JFileChooser.java:610) at Test.lambda$main$0(Test.java:21) at java.desktop/java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:308) at java.desktop/java.awt.EventQueue.dispatchEventImpl(EventQueue.java:773) at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:720) at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:714) at java.base/java.security.AccessController.doPrivileged(AccessController.java:399) at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:86) at java.desktop/java.awt.EventQueue.dispatchEvent(EventQueue.java:742) at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:203) at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124) at java.desktop/java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:113) at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:109) at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101) at java.desktop/java.awt.EventDispatchThread.run(EventDispatchThread.java:90)

---------- BEGIN SOURCE ----------
import java.io.File;
import javax.swing.JFileChooser;
import javax.swing.SwingUtilities;
import javax.swing.UIManager;

class Test3 {
  public static void main(String[] args) throws Exception {
    SwingUtilities.invokeAndWait(() -> {
      try {
        UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
        JFileChooser fd = new JFileChooser();
        fd.setCurrentDirectory(new File("C:\\Users"));
        fd.showOpenDialog(null);
      }
      catch (Exception e) {
        e.printStackTrace();
        System.exit(1);
      }
    });
  }
---------- END SOURCE ----------

FREQUENCY : always



Comments
A pull request was submitted for review. URL: https://git.openjdk.org/jdk22u/pull/150 Date: 2024-04-17 11:04:48 +0000
17-04-2024

Fix request: 22u This bug fix resolves an intermittent problem where using JFileChooser is impossible because an exception is thrown when files are sorted. Clean backport. Low risk. Pull request: https://github.com/openjdk/jdk22u/pull/150
17-04-2024

Changeset: 2fcb8168 Author: Alexey Ivanov <aivanov@openjdk.org> Date: 2024-04-09 13:19:41 +0000 URL: https://git.openjdk.org/jdk/commit/2fcb816858406f33cefef3164b2c85f9f996c7da
09-04-2024

A pull request was submitted for review. URL: https://git.openjdk.org/jdk/pull/18126 Date: 2024-03-05 17:01:45 +0000
05-03-2024

JDK-8140204 reports the same problem but that time we didn't have such an extensive analysis of the problem, nor did we have an environment to reproduce the problem. It was reported in Windows Vista. If there's an environment where the bug can be reproduced, we should collect more data to understand why there are two folders which refer to the same file system location. In other words, how is it possible that the Personal (Documents) folder has its shadow without `isPersonal` set to true.
04-03-2024

I couldn't reproduce the issue in the same way it fails. However, I created a test case using reflection which proves there's inconsistency in the implementation of the Comparable interface in the Win32ShellFolder2 class. The reporter's analysis is absolutely accurate. My test is inspired by the reporter's tests at https://forum.vassalengine.org/t/cannot-open-window-for-load-save-or-beginlogs/76730/2 I can't explain how the list of files ended up with two directories for `Documents`. In my test, I create a fake personal using reflection. There must be another bug in JDK code which allows such duplicate entries in the list of files on desktop.
04-03-2024

Using a localised version of Windows 11 could help to reproduce the bug. The folder names in the user's home always have English names (Documents, Pictures), yet the displayed name is localised. It is possible to create a folder with the same name as localised one in the user's home. Thus, there will be two folders with the same name as seen by the shell (yet different file system path).
22-02-2024

Went through https://forum.vassalengine.org/t/cannot-open-window-for-load-save-or-beginlogs/76730/2 & https://github.com/skylot/jadx/issues/1628. I am unable to create 2 folders with same name, but based on what observations are shared by submitter i am triaging it. Need more analysis on what we are doing in code.
14-09-2023

Additional Information from submitter =========================== I can't give you steps to reproduce the issue naturally. For that, you need a Windows machine where Win32ShellFolder2.getDesktop().listFiles() returns an array containing an element the same as one of the elements in topFolderList, and another element with the same path but not marked special. I don't know how that comes about, only that it does sometimes. However, if you're able to manually create Win32ShellFolder2 objects, you can do the following: 1. Make a Win32ShellFolder2 for the path "C:\Users\duduy\Documents" for which isSpecial() and isFileSystem() are true. Call this one a. 2. Make a Win32ShellFolder2 for the path "C:\Users\duduy\Documents" for which isSpecial() is false and isFileSystem() is true. Call this one b. 3. Make a Win32ShellFolder2 for the path "C:\Users\duduy" for which isSpecial() is false and isFileSystem() is true. Call this one c. 4. Mock out Win32ShellFolderManager2.getPersonal() to return a. Then you'll see the following things occur: a.compareTo(c) < 0 b.compareTo(c) > 0 a.compareTo(b) == 0 Those three things combined demonstrate that compareTo() violates its contract.
12-09-2023

Mail to submitter: ==================== Please share steps to reproduce the issue
31-03-2023

I was trying to reproduce this issue on Windows 11. I am unable to create two directories with the same name. Need instructions on how to create the directory structure to reproduce this issue.
31-03-2023

Additional Information from submitter =========================== I see that you weren't able to reproduce the bug from the test case, which doesn't surprise me. You can only reproduce the bug from the test case on a machine where you have a duplicate entry where one is a "top" folder an the other is not special. It should nonetheless be possible to see from examining the code where the bug occurs when you do have such duplicate entries.
30-03-2023

Checked with attached testcase in Windows 10, Issue is not reproducible, Test Result ========== 8u361: Pass jdk11: Pass jdk17: Pass jdk19: Pass jdk20: Pass jdk21ea15: Pass
28-03-2023