Thanks Stanimir,
On 9/12/14 11:24 AM, Stanimir Simeonoff wrote:
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;
}
});
On Cleaner:
Good point but this is a different issue.
I have logged
https://bugs.openjdk.java.net/browse/JDK-8058296
Thanks for pointing that out.
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.
I am still unconvinced this is worth the additional
complexity it would bring to the implementation.
The deprecated methods were using HashMap to store listeners,
and therefore the order in which listeners were invoked was also
undefined. Nobody has ever complained.
(http://hg.openjdk.java.net/jdk8u/jdk8u-dev/jdk/file/3ae82f0c6b31/src/share/classes/java/util/logging/LogManager.java#l1431)
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.
We could do that - invoke all listeners and rethrow the exception
caught by the first listener that failed (on the basis that if any other
listener subsequently fails it may be because the previous listener
has left the system in an inconsistent state, and that therefore
the first exception caught has 'more value' than the others).
I am not completely convinced this is a better behavior than
simply stopping invoking listeners at the first exception.
I wonder if there would be a good wording to say that listeners
are not supposed to throw, but that if they do, the exceptions
will be propagated...
best regards,
-- daniel
Stanimir
