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

Reply via email to