Theoretical data race on java.util.logging.Handler.sealed
peter.levart at gmail.com
Sun Dec 22 13:23:20 UTC 2013
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:
> 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.
> Digress: Just notice that the closeable handler classes are not
> AutoCloseable (they don't implement Closeable either). The close()
> method don't throw IOException but instead throws SecurityException an
> unchecked exception. Otherwise, we could use try-with-resources.
>> 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 things:
>> - 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.
> Yes I agree this is a bug that should get fixed.
>> 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?
> "localhost" should be fine and there are other tests depending on it
> be resolvable.
So should anything else be done before pushing this to jdk9/dev ?
More information about the core-libs-dev