Hi Mandy, Daniel,

Thanks for reviews. I just pushed this change to jdk9-dev/jdk ...

Regards, Peter

On 12/23/2013 05:50 AM, Mandy Chung wrote:

On 12/22/2013 5:23 AM, Peter Levart wrote:
Hi Mandy,

On 12/19/2013 10:38 PM, Mandy Chung wrote:
On 12/19/13 7:49 AM, Peter Levart wrote:
Hi Mandy, Daniel,

I didn't like the package-protected getters either. So here's another variant that replaces Handler.configure() method with a package-protected constructor which is chained from JDK subclasses:

http://cr.openjdk.java.net/~plevart/jdk8-tl/jul.Handler.sealed/webrev.07/


Looks good. Thanks for making the change and the new test. It'd be good to close the handlers by the test. The test is running in othervm mode and the Cleaner thread will close the handler when VM exits and the test is fine as it is.

Well, not really. The Cleaner only closes Handlers that are attached to Loggers but the test just instantiates Handlers and doesn't add them to any Loggers. It's harmless as it is, othervm will exit nevertheless and resources will be freed...

I tried closing Handlers at the end of test, but that requires "control" LoggingPermission and we don't want to run the test with "control" permission since we want to check that instantiating Handlers (SocketHandler too) doesn't require "control" permission.

Thanks and the test is fine as it is.


So should anything else be done before pushing this to jdk9/dev ?

Fix looks good and have a regression test. It's good to go and push to jdk9/dev. No other approval needed.

thanks
Mandy

Reply via email to