RFR: 8153955: java.util.logging.FileHandler can not create file synchronously over 101 access

[Daniel Fuchs]

Hi Ramanand,

Thanks for the updated. I still have some remarks:

FileHandler.java:

   98  *        specifies the maximum number of concurrent locks hold by
   99  *        FileHandler (defaults 100). </li>

Is the verb form correct: hold => held ?

logging.properties:

   42 # when the unique field %u is incremented as per the javadoc.

"... as per the javadoc" => "... as per FileHandler API documentation"

FileHandlerMaxLocksTest.java:

- FileHandlerMaxLocksTest::createLoggerDir():

   75         String tmpDir = System.getProperty("java.io.tmpdir");
   76         if (tmpDir == null) {
   77             tmpDir = System.getProperty("user.home");
   78         }
   79         File tmpOrHomeDir = new File(tmpDir);
   80         File loggerDir = new File(tmpOrHomeDir, LOGGER_DIR);

The preferred place for a test to create file is the scratch
directory that jtreg creates for the test. The scratch directory
location is available from the "user.dir" system property.

I suggest changing the code above to:

    String userDir =   System.getProperty("user.dir", ".");
    File loggerDir = new File(userDir, LOGGER_DIR);

- FileHandlerMaxLocksTest::main

I am not sure what you try to achieve by using WeakReference there.
I would suggest to store all created FileHandler instances into a list,
which would allow you to close them properly in the finally clause
just before deleting LOGGER_DIR.

Different operating systems might handle lock files differently.
I am afraid that some operating system might not let you delete
a directory that contains a file which is still locked by the
system.

best regards,

-- daniel

On 09/06/16 06:40, Ramanand Patil wrote:
> Hi,
>
> Please review the updated Webrev at: http://cr.openjdk.java.net/~rpatil/8153955/webrev.01/
>
> FileHander.java and logging.properties files are updated as per Daniel's suggestion.
>
>
> Regards,
> Ramanand.
>
>
> -----Original Message-----
> From: Daniel Fuchs
> Sent: Wednesday, June 08, 2016 6:57 PM
> To: Ramanand Patil; core-libs-dev at openjdk.java.net
> Subject: Re: RFR: 8153955: java.util.logging.FileHandler can not create file synchronously over 101 access
>
> Hi Ramanand,
>
> Thanks for looking into this.
>
> 1. FileHander.java:
>
>    94  * <li>   <handler-name>.append
>    95  *        specifies whether the FileHandler should append onto
>    96  *        any existing files (defaults to false). </li>
>    97  * <li>   java.util.FileHandler.maxLocks
>    98  *        specifies the maximum number of concurrent locks hold by
>    99  *        FileHandler (defaults 100). </li>
>
> line 97: a) this is java.util.logging.FileHandler, not java.util.FileHandler.
>           b) for consistency with the specification of other FileHandler
>              properties I suggest to change this line to:
>
>    97  * <li>   <handler-name>.maxLock
>
> 2. logging.properties:
>
>    31 # Default number of locks FileHandler can obtain synchronously.
>    32 # This specifies maximum number of concurrent locks by FileHandler
>    33 # when the unique field %u is incremented as per the javadoc.
>    34 java.util.logging.FileHandler.maxLocks = 100
>
> Please move lines 31-34 between lines 44 and 45: that's where they belong.
>
> best regards,
>
> -- daniel
>
>
>
>
> On 08/06/16 12:06, Ramanand Patil wrote:
>> Hi all,
>>
>> Please review the following bug fix:
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8153955
>>
>> Webrev: http://cr.openjdk.java.net/~rpatil/8153955/webrev.00/
>>
>> Fix: A new configurable java property- "java.util.logging.FileHandler.maxLocks" is added to the FileHandler which can be set in the custom logger config file. The default value of the this property will be mentioned in the default config file.(logging.properties).
>>
>>
>>
>>
>>
>> Regards,
>>
>> Ramanand.
>>
>