carterkozak commented on a change in pull request #434: URL: https://github.com/apache/logging-log4j2/pull/434#discussion_r516721169
########## File path: log4j-core/src/main/java/org/apache/logging/log4j/core/util/DefaultShutdownCallbackRegistry.java ########## @@ -87,42 +92,41 @@ public void run() { } private static class RegisteredCancellable implements Cancellable { - // use a reference to prevent memory leaks - private final Reference<Runnable> hook; - private Collection<Cancellable> registered; + private Runnable callback; + private Collection<Reference<Cancellable>> registered; - RegisteredCancellable(final Runnable callback, final Collection<Cancellable> registered) { + RegisteredCancellable(final Runnable callback, final Collection<Reference<Cancellable>> registered) { + this.callback = callback; this.registered = registered; - hook = new SoftReference<>(callback); } @Override public void cancel() { - hook.clear(); - registered.remove(this); + callback = null; + registered.removeIf(ref -> ref.get() == this); Review comment: I'm going to update this to include the null check to remove collected values, and add a null check on registered so that subsequent `cancel()` invocations do not NPE. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org