Zombie FileHandler locks can exhaust all available log file locks.
jason_mehrens at hotmail.com
Tue Jun 24 20:55:20 UTC 2014
I agree with you. I think you have it all covered.
Thanks for working on this!
> Date: Tue, 24 Jun 2014 19:37:24 +0200
> From: daniel.fuchs at oracle.com
> To: jason_mehrens at hotmail.com; alan.bateman at oracle.com; core-libs-dev at openjdk.java.net
> Subject: Re: Zombie FileHandler locks can exhaust all available log file locks.
> Hi Jason,
> Thanks for the feedback!
> On 6/24/14 7:08 PM, Jason Mehrens wrote:
>> With regard to JDK-4420020, in the original webrev Jim was incorrectly using java.io.File.canWrite() but that webrev was replaced by the current version. The NIO.2 code performs the effective access checks correctly.
>> With regard to JDK-6244047 my concern was that checking the permissions and preconditions up front is a small 'TOCTOU' race and redundant because the next step after that is to open the FileChannel. I would assume both CREATE or CREATE_NEW plus WRITE would perform the effective access checks on open.
>> The use of delete on exit is a deal breaker because you can't undo a registration on close of the FileHandler which can trigger an OOME. See https://bugs.openjdk.java.net/browse/JDK-4872014. I've used custom proxy handlers that generate a file name based off of the LogRecord which results in generating lots of file names and opening and close the FileHandler on demand.
> ah. hmmmm. I didn't think there would be that many FileHandlers...
> Well - I guess that if we find a way to reuse the zombie
> lock files then we don't really need the delete on exit.
> Someone creating a FileHandler programmatically should be responsible
> for closing it - so if an application creates FileHandlers without
> closing them then it's a bug in the application.
>> If the behavior is reverted to use CREATE instead of CREATE_NEW then we have to deal with JDK-8031438 because any user code can lock a file ahead of opening the FileHandler.
> Yes - I discovered that while writing a test for my suggested fix ;-)
> Catching OverlappingFileLockException in FileHandler.openFiles
> and setting available to false when it's thrown fixes the issue.
> So let's just remove the deleteOnExit & add the catch for
> OverlappingFileLockException to my patch and we should be
> On the other hand we could just use replace CREATE_NEW by
> CREATE but I have some misgivings about the part that
> sets available to true when tryLock throws an IOException.
> I'm not sure we should be doing that if we haven't actually
> created the lock file. So I think I'd prefer keeping the
> two steps behavior - first try CREATE_NEW - then attempt
> to use CREATE if CREATE_NEW failed...
> I'm not sure CREATE will check the access rights for writing
> in the directory if the file already exists - so checking
> the directory for write access in that case is probably
> the right thing to do.
> Would you agree?
> -- daniel
>>> Date: Mon, 23 Jun 2014 17:13:04 +0200
>>> From: daniel.fuchs at oracle.com
>>> To: Alan.Bateman at oracle.com; jason_mehrens at hotmail.com; core-libs-dev at openjdk.java.net
>>> Subject: Re: Zombie FileHandler locks can exhaust all available log file locks.
>>> On 6/23/14 4:53 PM, Alan Bateman wrote:
>>>> On 23/06/2014 10:48, Daniel Fuchs wrote:
>>>>> All in all - I feel our best options would be to revert to using
>>>>> CREATE, or use CREATE_NEW + DELETE_ON_CLOSE, or not do anything
>>>>> and live with the issue.
>>>>> Hopefully some nio experts will chime in ;-)
>>>> The logging use of FileLock is a bit unusual but looking at it now then
>>>> I don't see the reason why it needs to use CREATE_NEW.
>>> OK - thanks - in the discussion that ended up in the patch where
>>> CREATE_NEW was introduced Jason (I think it was Jason) pointed at
>>> I'm guessing here that maybe it would be wrong to use an already
>>> existing lock file if you don't have the rights to write to
>>> its directory - and that using CREATE_NEW ensures that you have
>>> all necessary access rights all along the path.
>>>> I don't think
>>>> DELETE_ON_CLOSE is suitable here as that work is for transient work
>>>> files which isn't the case here. Instead it could use deleteOnExit to
>>>> have some chance of removing it on a graceful exit.
>>> Right - I suggested a patch in another mail where I just use that.
>>>> BTW: Do you know why locks is Map? Would a Set do?
>>> It certainly looks as if we could use a simple HashSet here.
>>> Thanks Alan!
>>> -- daniel
More information about the core-libs-dev