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

[Peter Levart]

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

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
>