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

[Paul Sandoz]

On Sep 10, 2014, at 1:42 PM, Daniel Fuchs <daniel.fuchs at oracle.com> 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.
> 
> One nice thing about ConfigurationListener being an abstract class is
> that I don't need to invent an 'IdentityCopyOnWriteArrayList' which
> is what I would prefer to use if equals() could be overriden by
> implementations.
> 

FWIW i think you could do:

  removeIf(e -> listener == e);

Idiomatically callback methods are often prefixed with on:

  onConfigurationLoaded.

My inclination is this should be an interface if at all possible. The permission check on construction seems odd to me but i don't understand the security requirements here. It's very tempting to suggest not defining any new interface and reuse Runnable, since as you indicate below this is not expected to be common functionality and there is likely to be a single instance registered. 

Paul.


> Also I like it that you need to have the LoggingPermission("Control")
> to subclass ConfigurationListener.
> 
> In practice - I believe that there aren't that many application
> which need this functionality - and I also believe that for those
> that need it then there will be a single ConfigurationListener
> that will be registered only once, and therefore having an
> abstract class for ConfigurationListener shouldn't be that much of
> a hassle for them.
>