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@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>   &lt;handler-name&gt;.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>   &lt;handler-name&gt;.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.



Reply via email to