Commons IO
  1. Commons IO
  2. IO-216

Delete files quietly when an exception is thrown during initialization

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.4
    • Fix Version/s: 2.0
    • Component/s: Streams/Writers
    • Labels:
      None

      Description

      LockableFileWriter fails to report lock file deletion failure - it calls lockFile.delete() several times but fails to check the return code.

      N.B. IIRC, file.delete() returns false if there was no file to delete, so any fix needs to take this into account.

        Activity

        Hide
        Niall Pemberton added a comment -

        OK I see you updated the JavaDoc:

        http://svn.apache.org/viewvc?view=revision&revision=1002182

        so resolving this as FIXED(again!)

        Show
        Niall Pemberton added a comment - OK I see you updated the JavaDoc: http://svn.apache.org/viewvc?view=revision&revision=1002182 so resolving this as FIXED(again!)
        Hide
        Sebb added a comment -

        The problem with ignoring the deletion failure is that the stale lock file may be detected until much later when an attempt to lock a file fails unexpectedly.

        The exception does give the name of the lockFile, but it could be difficult to determine where it was created, and indeed whether it is still in use.

        Not sure how best to solve this.

        In the meantime, I propose to update the Javadoc to mention that the lock file could theoretically be left behind after close or on init failure.

        Show
        Sebb added a comment - The problem with ignoring the deletion failure is that the stale lock file may be detected until much later when an attempt to lock a file fails unexpectedly. The exception does give the name of the lockFile, but it could be difficult to determine where it was created, and indeed whether it is still in use. Not sure how best to solve this. In the meantime, I propose to update the Javadoc to mention that the lock file could theoretically be left behind after close or on init failure.
        Hide
        Niall Pemberton added a comment -

        OK I've applied the patch I suggested and renamed this ticket to reflect that change.

        Show
        Niall Pemberton added a comment - OK I've applied the patch I suggested and renamed this ticket to reflect that change.
        Hide
        Niall Pemberton added a comment -

        lockFile.delete() is called three times. Two of those are in the initWriter() method, trying to clean up when an exception is thrown. I assume you mean by "report" to throw an exception - but that would hide the underlying exception that is re-thrown later. In fact I think we should use the FileUtils.deleteQuietly() method here to avoid that (attaching a patch for that).

        The other time is in the close() method. We could throw an exception here, but I'm not sure about that - since although it is a failure it has done its job.

        Anyway, perhaps you could put forward a more concrete proposal about how this should be resolved.

        Show
        Niall Pemberton added a comment - lockFile.delete() is called three times. Two of those are in the initWriter() method, trying to clean up when an exception is thrown. I assume you mean by "report" to throw an exception - but that would hide the underlying exception that is re-thrown later. In fact I think we should use the FileUtils.deleteQuietly() method here to avoid that (attaching a patch for that). The other time is in the close() method. We could throw an exception here, but I'm not sure about that - since although it is a failure it has done its job. Anyway, perhaps you could put forward a more concrete proposal about how this should be resolved.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development