[GitHub] [logging-log4j2] carterkozak commented on a change in pull request #434: LOG4J2-2954 Retain strong reference to shutdown callbacks

[GitBox]

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