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

Stanimir Simeonoff stanimir at riflexo.com
Fri Sep 12 09:24:04 UTC 2014


>
> Good point. But I expect the concrete listener class will also be usually
> loaded by the context class loader - so isn't that kind of expected anyway?
> I didn't want to stuff too many implementation details into the spec.
> Maybe that could be an API note however.
> Would anyone be able to suggest a good wording?
>


On leaks. The c-tor of LogManager leaks the thread group and the current
AccessControlContext till JVM shutdown. The constructor code should be
something like the one below with Cleaner getting ThreadGroup in its c-tor
as well.


    java.security.AccessController.doPrivileged(//zip
thread.inheritedAccessControlContext
        new java.security.PrivilegedAction<Void>() {
        public Void run() {
            // put the cleaner in the systsm threadgroup
            ThreadGroup grp =
Thread.currentThread().getThreadGroup();
            for(ThreadGroup parent;null!=(parent = grp.getParent());) {
                grp = parent;
            }

            Cleaner cleaner = new Thread(grp);
            cleaner.setContextClassLoader(null);//zip the classloader

            // Add a shutdown hook to close the global handlers.
            try {
                Runtime.getRuntime().addShutdownHook(cleaner);
            } catch (IllegalStateException e) {
                // If the VM is already shutting down,
                // We do not need to register shutdownHook.
            }

            return null;
        }
    });


 Also if there is a runtime exception during run() of a listener it will
>> block any other listeners to be invoked and the exception is going to be
>> propagated past readConfiguration(). I suppose that's ok however the
>> listeners are to be invoked in a 'random' order depending on their
>> identityHashCode. So if there is an exception in the last registered
>> there is no guarantee to invoke even the 1st added listener. The entire
>> process is not deterministic which is not ok for listeners invocation.
>>
>
> The 'old' deprecated code was already like that.
> This doesn't mean we shouldn't try to make it better - but I have
> to ask whether it's worth the trouble.
>
> However the listeners are to be invoked in the same order they have been
added.


> Are there applications where there will be several listeners?
>

Not sure, however listeners can be added at any time.
readConfiguration(InputStream) is public too, so it's plausible.


> And if there are - shouldn't these listener run their code in a
> try { } catch ( ) { } to ensure that the exception will be
> logged - or rather reported - where it belong?
>

IMO, the API can't demand any cooperation from user space code .


> IMO the only thing we could do here is to silently drop any
> runtime/exception or error, which I am a bit reluctant to do;
> I believe it's the concrete implementation of the listener
> which should take care of that.
>
> I'd rather run the listeners in try/catch and throw the 1st exception at
the end.
Defining the 1st however requires obeying the order the listeners have been
added.
As far as I see there is no documentation how the listeners are supposed to
behave or be invoked.


Stanimir



More information about the core-libs-dev mailing list