Theoretical data race on java.util.logging.Handler.sealed

[Daniel Fuchs]

On 12/14/13 6:38 PM, Peter Levart wrote:
> Hi,
>
> Daniel reminded me of a couple of issues the 4th revision of the patch
> would have when backporting to 7u. So here's another variant that tries
> to be more backport-friendly:
>
> http://cr.openjdk.java.net/~plevart/jdk8-tl/jul.Handler.sealed/webrev.05/
>
> This variant could be backported by simply replacing a limited variant
> of doPrivileged (introduced in JDK 8) with full variant and still not
> elevate the privilege of Socket creation in SocketHandler. I also
> removed the need to subclass various ConfigureAction(s) with annonymous
> inner subclasses by introducing overloaded constructors to
> ConfigureActions(s) that follow the overloaded constructors of various
> Handlers.
>
> Regards, Peter

Hi Peter,

This looks good! Particularly since moving connect() out of
the doProvileged has removed the need for wrapping/unwrapping the
IOException.

best regards,

-- daniel

>
> On 12/14/2013 12:25 PM, Peter Levart wrote:
>> Hi Mandy,
>>
>> On 12/13/2013 12:37 AM, Mandy Chung wrote:
>>> Hi Peter,
>>>
>>> On 12/8/2013 11:19 AM, Peter Levart wrote:
>>>> H Mandy,
>>>>
>>>> I created an issue for it nevertheless:
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8029781
>>>>
>>>> You're right, doPrivileged() is a more straight-forward approach
>>>> than 'sealed' variable. Since this might only be considered for
>>>> inclusion in JDK9 when lambdas are already a tried technology, how
>>>> do you feel about using them for platform code like logging? As far
>>>> as I know (just checked), lambda meta-factory is not using any
>>>> j.u.logging, so there is no danger of initialization loops or similar:
>>>>
>>>> http://cr.openjdk.java.net/~plevart/jdk8-tl/jul.Handler.sealed/webrev.03/
>>>>
>>>
>>> Sorry for the delay to get to this.
>>
>> No rush. We have time before JDK9 gets set-up and running...
>>
>>>
>>> Alan is right that java.lang.invoke.ProxyClassesDumper does use
>>> PlatformLogger which will forward calls to j.u.logging if
>>> -Djava.util.logging.config.file is set or java.util.logging has been
>>> initialized (via other j.u.logging call).  It means that it may lead
>>> to recursive initialization.  Also the current PlatformLogger
>>> implementation formats the message in the same way as j.u.logging
>>> that may load locale providers and other classes.  I am afraid there
>>> are issues to tease out and resolve.
>>
>> It's unfortunate that a lambda debugging feature prevents us from
>> using a basic language feature in j.u.logging code. As far as I know,
>> java.lang.invoke.ProxyClassesDumper is only used if
>> 'jdk.internal.lambda.dumpProxyClasses' system property is set to point
>> to a directory where lambda proxy class files are to be dumped as they
>> are generated - a debugging hook therefore. Wouldn't it be good-enough
>> if error messages about not-being able to set-up/use the dump facility
>> were output to System.err directly - not using PlatformLogger at all?
>>
>>>
>>> The overloads the doPrivileged method makes it cumbersome to use
>>> lambda that causes you to workaround it by adding a new
>>> PrivilegedVoidAction interface which is clever.  So I think it isn't
>>> too bad for this patch to use anonymous inner class and have the
>>> doPrivileged call in the constructor.
>>
>> Right. I have prepared a modified webrev which doesn't use lambdas:
>>
>> http://cr.openjdk.java.net/~plevart/jdk8-tl/jul.Handler.sealed/webrev.04/
>>
>> In attempt to minimize the boilerplate, I haven't just replaced
>> lambdas with anonymous inner classes, but rather turned all private
>> configure() methods into ConfigureAction inner classes. In two
>> occasions (SocketHandler and StreamHandler), they are extended with
>> anonymous inner classes to append some actions. In SocketHandler I
>> kept the mechanics of transporting the checked IOException out of
>> PrivilegedAction by wrapping it with Unchecked IOException and later
>> unwrapping and throwing it, rather than using
>> PrivilegedExceptionAction which would further complicate exception
>> handling, since it declares throwing a general j.l.Exception, but the
>> SocketHandler constructor only declares throwing IOException...
>>
>> I think this could be backported to 7u as-is.
>>
>> Regards, Peter
>>
>>>
>>>
>>> Mandy
>>
>