Theoretical data race on java.util.logging.Handler.sealed
daniel.fuchs at oracle.com
Thu Dec 19 16:24:07 UTC 2013
Good idea to add a package constructor in Handler.
It looks much cleaner than the configure method.
Good tests too - thanks for adding that!
To access files with JPRT there is a simpler manner (though what
you have looks good).
JPRT sets a system property "test.src" which point at the
directory where the sources are located.
So you could have used something like:
new File(System.getProperty("test.src", "."),
On 12/19/13 4:49 PM, Peter Levart wrote:
> On 12/18/2013 11:55 PM, Mandy Chung wrote:
>> On 12/18/2013 9:03 AM, Peter Levart wrote:
>>> Hi Mandy, Daniel,
>>> Here's yet another variant that reduces the doPrivileged code to just
>>> Handler's setters. This way no LogManager methods are invoked under
>>> elevated privilege:
>> This version looks good. I like the refactoring to have the subclass
>> to call the common code Handler.configure method. It may be better to
>> have the configure method (or a new one) that takes the default Level
>> and default Formatter instead of the package-private getters.
>> I don't see why the handler constructors are designed to call the
>> overridden methods rather than the initialization and if a subclass
>> has its custom field, it should initialize its custom fields in its
>> constructor implementation. Anyway this would be a separate clean
>> up task from this one.
>> Can you also add a sanity test to verify that these handlers can be
>> constructed successfully with a security manager installed?
> 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:
> I filed another bug that is fixed by this patch:
> And I created a test (see webrev.07) that almost passes when run against
> unchanged JDK 8 (the failure is just at the end when calling new
> SocketHandler(host, port) - access denied
> ("java.util.logging.LoggingPermission" "control")). If I comment-out the
> System.setSecurityManager() from the test, it passes with unchanged
> code. This is to verify the test itself. When run against the patched
> JDK 8, it passes even when SecurityManager is active - this verifies two
> - the behaviour of patched code is analogous to unpatched code as far as
> defaults and configured handler properties is concerned and it conforms
> to javadoc
> - the patched code does not require any new permissions - it actually
> requires less, because it fixes bug 8030801.
> All java/util/logging jtreg tests pass with patched code. I hope that
> "localhost" is a resolvable name on all environments and that new
> ServerSocket(0) creates a server socket bound at least to the IP address
> that "localhost" resolves to. Is this reasonable to assume?
> Regards, Peter
More information about the core-libs-dev