Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8305072

Win32ShellFolder2.compareTo is inconsistent

XMLWordPrintable

        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


              aivanov Alexey Ivanov
              webbuggrp Webbug Group
              Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

                Created:
                Updated:
                Resolved: