Review Request (1-line fix): 7032589 "FileHandler leaking file descriptor of the file lock"

Mandy Chung mandy.chung at oracle.com
Fri Apr 15 19:12:05 UTC 2011


  Hi Remi,

On 04/15/11 11:05, Rémi Forax wrote:
> Hi Mandy,
> if fc.close() throws an IOException , it goes wrong.
> I think you need to put fc.close() in its own a try/catch(IOException).
>

That's a good point.  I think it should throw IOException if anything 
goes wrong with fc.close(). Revised the fix:
     http://cr.openjdk.java.net/~mchung/jdk7/7032589/webrev.01/

The existing implementation looks a bit suspicious as it silently 
ignores the IOException thrown by tryLock() and uses that lock file.  I 
am inclined to leave the existing behavior as it is to minimize 
behavioral change.  And just resolve this file descriptor leak issue in 
this fix. I'll open a bug to follow up this.

  412                 try {
  413                     FileLock fl = fc.tryLock();
  414                     if (fl == null) {
  415                         // We failed to get the lock.  Try next file.
  416                         continue;
  417                     }
  418                     // We got the lock OK.
  419                 } catch (IOException ix) {
  420                     // We got an IOException while trying to get the lock.
  421                     // This normally indicates that locking is not supported
  422                     // on the target directory.  We have to proceed without
  423                     // getting a lock.   Drop through.
  424                 }

Mandy





More information about the core-libs-dev mailing list