Daniel,

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.


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.

Jason



----------------------------------------
> Date: Mon, 23 Jun 2014 17:13:04 +0200
> From: daniel.fu...@oracle.com
> To: alan.bate...@oracle.com; jason_mehr...@hotmail.com; 
> core-libs-dev@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
> https://bugs.openjdk.java.net/browse/JDK-4420020
>
> 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
>
>>
>> -Alan.
>>
>                                         

Reply via email to