Pivot
  1. Pivot
  2. PIVOT-756

FileBrowserSheet displays incorrect directory listing.

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.0.1
    • Component/s: wtk-terra
    • Labels:
      None
    • Environment:
      Windows 7 Professional, Sun JRE 1.6.0_25

      Description

      I've ran into intermittent issues with FileBrowserSheet displaying a incorrect directory listing.

      The symptoms that I see are:

      • The file list includes files/directories from user.home (despite setRootDirectory(somewhereElse) being called prior to the open().)
      • The file list has duplicate entries.

      It's reproducible with a short test case on my system, so I highly doubt that it's a issue with my application code.

      1. FileBrowserSheetTest.java
        3 kB
        Yawning Angel
      2. FileBrowserSheetFix.diff
        6 kB
        Yawning Angel
      3. FileBrowserSheet.png
        29 kB
        Yawning Angel

        Activity

        Hide
        Greg Brown added a comment -

        Patch applied. Thanks!

        Show
        Greg Brown added a comment - Patch applied. Thanks!
        Hide
        Yawning Angel added a comment -

        > That should work fine. But if you do resolve the issue, please consider submitting a patch with the fix. You can attach it to this issue. Thanks!

        Attached, your suggestion worked perfectly. Thanks for your assistance in this matter.

        Show
        Yawning Angel added a comment - > That should work fine. But if you do resolve the issue, please consider submitting a patch with the fix. You can attach it to this issue. Thanks! Attached, your suggestion worked perfectly. Thanks for your assistance in this matter.
        Hide
        Yawning Angel added a comment -

        Patch that implements Greg Brown's suggestion.

        Show
        Yawning Angel added a comment - Patch that implements Greg Brown's suggestion.
        Hide
        Greg Brown added a comment -

        > I take it there won't be any problems if I included a corrected copy of TerraFileBrowserSkin, and call Theme.getTheme().set() in my app's startup method?

        That should work fine. But if you do resolve the issue, please consider submitting a patch with the fix. You can attach it to this issue. Thanks!

        Show
        Greg Brown added a comment - > I take it there won't be any problems if I included a corrected copy of TerraFileBrowserSkin, and call Theme.getTheme().set() in my app's startup method? That should work fine. But if you do resolve the issue, please consider submitting a patch with the fix. You can attach it to this issue. Thanks!
        Hide
        Yawning Angel added a comment -

        That's odd, the sample does call close() before spawning a new sheet, sorry for the confusion with the shoddy sample, I couldn't think of a clever way to demonstrate odd behavior quickly since there doesn't appear to be a way for a application to pull out the directory listing contents.

        I take it there won't be any problems if I included a corrected copy of TerraFileBrowserSkin, and call Theme.getTheme().set() in my app's startup method? I could just ship a corrected JAR, but I'm somewhat hesitant to do that for logistical reasons.

        Show
        Yawning Angel added a comment - That's odd, the sample does call close() before spawning a new sheet, sorry for the confusion with the shoddy sample, I couldn't think of a clever way to demonstrate odd behavior quickly since there doesn't appear to be a way for a application to pull out the directory listing contents. I take it there won't be any problems if I included a corrected copy of TerraFileBrowserSkin, and call Theme.getTheme().set() in my app's startup method? I could just ship a corrected JAR, but I'm somewhat hesitant to do that for logistical reasons.
        Hide
        Greg Brown added a comment -

        FWIW, on my system, multiple file browser sheets are opened before I get the "Race triggered after..." message (on the last run, I got 14).

        But after reviewing the code, I do see a potential race condition. The execute() method of RefreshFileListTask is actually populating the table view. This should be done by the TaskListener that invokes the task, in refreshFileList(). execute() is called on the background thread so it should not be accessing the user interface directly.

        RefreshFileListTask should probably be updated to return File[] instead of Void, and the rest of the code in execute() should be moved to the anonymous implementation of taskExecuted() in refreshFileList().

        Show
        Greg Brown added a comment - FWIW, on my system, multiple file browser sheets are opened before I get the "Race triggered after..." message (on the last run, I got 14). But after reviewing the code, I do see a potential race condition. The execute() method of RefreshFileListTask is actually populating the table view. This should be done by the TaskListener that invokes the task, in refreshFileList(). execute() is called on the background thread so it should not be accessing the user interface directly. RefreshFileListTask should probably be updated to return File[] instead of Void, and the rest of the code in execute() should be moved to the anonymous implementation of taskExecuted() in refreshFileList().
        Hide
        Yawning Angel added a comment -

        Screenshot of incorrect behavior.

        Show
        Yawning Angel added a comment - Screenshot of incorrect behavior.
        Hide
        Yawning Angel added a comment -

        Errr, this only uses a single FileBrowserSheet (I call close(), immediately after the open()).

        The problem is intermittent (I think it's a race condition, from looking at the code populating the directory listing is done by a separate task.).

        What the sample does is:
        a) Create a new FileBrowserSheet.
        b) Set the root directory to the current working directory. (Anywhere that's not user.home is sufficient.)
        c) Set a disabled file filter that will break out of the loop, and raise a error, break the loop, if it sees files not in the current working directory.
        d) Opens and closes the FileBrowserSheet.

        I'm not sure if this is the best way to demonstrate it, but what happens in my app (or if I open a FileBrowserSheet) is that once in a while, despite calling setRootDirectory(<Somewhere other than user.home>), the directory listing contains files from my ~ in addition to files from <Somewhere other than user.home>. More rarely, I see "all files are from the correct directory, but everything is duplicated" (Eg: ~/tmp/foo.xml is listed twice).

        Even if I don't use a loop here, and just have a simple app with a button that opens a FileBrowserSheet after a setRootDirectory(somewhereElse) call, I can get the problem to appear eventually with enough manual clicking. But since that is tedious and annoying, I used a loop and a automated way to detect that something anomalous has happened.

        Show
        Yawning Angel added a comment - Errr, this only uses a single FileBrowserSheet (I call close(), immediately after the open()). The problem is intermittent (I think it's a race condition, from looking at the code populating the directory listing is done by a separate task.). What the sample does is: a) Create a new FileBrowserSheet. b) Set the root directory to the current working directory. (Anywhere that's not user.home is sufficient.) c) Set a disabled file filter that will break out of the loop, and raise a error, break the loop, if it sees files not in the current working directory. d) Opens and closes the FileBrowserSheet. I'm not sure if this is the best way to demonstrate it, but what happens in my app (or if I open a FileBrowserSheet) is that once in a while, despite calling setRootDirectory(<Somewhere other than user.home>), the directory listing contains files from my ~ in addition to files from <Somewhere other than user.home>. More rarely, I see "all files are from the correct directory, but everything is duplicated" (Eg: ~/tmp/foo.xml is listed twice). Even if I don't use a loop here, and just have a simple app with a button that opens a FileBrowserSheet after a setRootDirectory(somewhereElse) call, I can get the problem to appear eventually with enough manual clicking. But since that is tedious and annoying, I used a loop and a automated way to detect that something anomalous has happened.
        Hide
        Greg Brown added a comment -

        It is not clear exactly what this test app is supposed to demonstrate. Can you describe the issue more specifically?

        Also, the app appears to attempt to open 1000 file browser sheets simultaneously. This may work, though it is not a supported use case. Can you update the app to demonstrate the problem using a single FileBrowserSheet?

        Show
        Greg Brown added a comment - It is not clear exactly what this test app is supposed to demonstrate. Can you describe the issue more specifically? Also, the app appears to attempt to open 1000 file browser sheets simultaneously. This may work, though it is not a supported use case. Can you update the app to demonstrate the problem using a single FileBrowserSheet?
        Hide
        Yawning Angel added a comment -

        Simple test case.

        Show
        Yawning Angel added a comment - Simple test case.

          People

          • Assignee:
            Unassigned
            Reporter:
            Yawning Angel
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development