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

[Bernd Eckenfels]

Hello,

I find the "concurrent/synchronous" comment a bit confusing.

How about:

# Number of attempts to obtain lock file in FileHandler
# Implemented by incrementing the %u placeholder as documented
# in FileHandler Javadoc

In the test I would add a comment

"200 raises the default limit of 100, we try 102 times"



Gruss
Bernd


Am Thu, 9 Jun 2016 07:31:25 +0100
schrieb Daniel Fuchs <daniel.fuchs at oracle.com>:

> 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.
> >>
> >
>