RFR: 8043306 - Provide a replacement for the API that allowed to listen for LogManager configuration changes

[Peter Levart]

On 09/11/2014 06:17 PM, Daniel Fuchs wrote:
> On 9/10/14 1:25 PM, Alan Bateman wrote:
>> On 10/09/2014 10:49, Daniel Fuchs wrote:
>>> Hi,
>>>
>>> Please find below a proposed patch for
>>>
>>> 8043306 - Provide a replacement for the API that allowed to listen
>>>           for LogManager configuration changes
>>> https://bugs.openjdk.java.net/browse/JDK-8043306
>>>
>>> Proposed Patch:
>>> http://cr.openjdk.java.net/~dfuchs/webrev_8043306/webrev.00/
>>>
>> It would be nice if ConfigurationListener could be an interface. I
>> realize this might mean looking again at the security concerns in this
>> area again to see if we could get away without requiring a construction
>> time permission check. Clearly if it could be an interface then it begs
>> the question as to why it's just not a Runnable but that probably means
>> yet more effort to figure out the right security and whether the access
>> control context should be recorded when registering.
>
> Given that both Paul & Alan have indicated that they would prefer
> to use interfaces for listeners, I have been working on a
> version that uses 'Runnable'.
>
> Thanks to Paul & Alan for all the suggestions that went into
> this new patch (especially for refining the spec and suggesting
> a nice way to rewrite addConfigurationListener() using lambdas)!
>
> http://cr.openjdk.java.net/~dfuchs/webrev_8043306/webrev.03/
>
> best regards,
>
> -- daniel

Hi Daniel,

Just a question about security and delayed execution...

If at the time the configuration listener is added to the LogManager, 
SecurityManager is not set, the listener will be invoked directly even 
if at time the listener is invoked, SM has been set. For example:


         LogManager.getLogManager().addConfigurationListener(...);
         ...
         System.setSecurityManager(...);
         ...
         // then somewhere in privileged context...
         LogManager.getLogManager().readConfiguration(...);


Wouldn't it be cleaner to evaluate SM presence in the lambda wrapper 
which would always be constructed, like that:


     public LogManager addConfigurationListener(Runnable listener) {
         final Runnable r = Objects.requireNonNull(listener);
         checkPermission();
         final AccessControlContext acc = AccessController.getContext();
         final Runnable pr = () -> {
             if (System.getSecurityManager() == null) {
                 r.run();
             } else {
AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
                     r.run();
                     return null;
                 }, acc);
             }
         };
         // Will do nothing if already registered.
         listeners.putIfAbsent(r, pr);
         return this;
     }


Regards, Peter