You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ro...@apache.org on 2018/06/13 15:00:34 UTC

[sling-org-apache-sling-commons-threads] 02/02: SLING-7526 - NPE in DefaultThreadPool$LoggingThreadLocalChangeListener.changed

This is an automated email from the ASF dual-hosted git repository.

rombert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-commons-threads.git

commit 90e02d020644ef1a2b5852171a59c6cbe84f5857
Author: Robert Munteanu <ro...@apache.org>
AuthorDate: Wed Jun 13 17:57:18 2018 +0300

    SLING-7526 - NPE in DefaultThreadPool$LoggingThreadLocalChangeListener.changed
    
    Only generate the (relatively expensive) ThreadLocal diff events if the
    listener is actually enabled.
---
 .../org/apache/sling/commons/threads/impl/DefaultThreadPool.java    | 5 +++++
 .../sling/commons/threads/impl/ThreadLocalChangeListener.java       | 2 ++
 .../org/apache/sling/commons/threads/impl/ThreadLocalCleaner.java   | 6 ++++--
 .../threads/impl/ThreadPoolExecutorCleaningThreadLocalsTest.java    | 1 +
 4 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/main/java/org/apache/sling/commons/threads/impl/DefaultThreadPool.java b/src/main/java/org/apache/sling/commons/threads/impl/DefaultThreadPool.java
index 604092a..e60cf97 100644
--- a/src/main/java/org/apache/sling/commons/threads/impl/DefaultThreadPool.java
+++ b/src/main/java/org/apache/sling/commons/threads/impl/DefaultThreadPool.java
@@ -172,6 +172,11 @@ public class DefaultThreadPool
             LOGGER.debug("Thread '{}' {} ThreadLocal {} with value {}", thread, mode, 
                     threadLocal != null ? threadLocal.getClass() : "<null>", value);
         }
+        
+        @Override
+        public boolean isEnabled() {
+            return LOGGER.isDebugEnabled();
+        }
     }
 
     /**
diff --git a/src/main/java/org/apache/sling/commons/threads/impl/ThreadLocalChangeListener.java b/src/main/java/org/apache/sling/commons/threads/impl/ThreadLocalChangeListener.java
index 27954f2..72237da 100644
--- a/src/main/java/org/apache/sling/commons/threads/impl/ThreadLocalChangeListener.java
+++ b/src/main/java/org/apache/sling/commons/threads/impl/ThreadLocalChangeListener.java
@@ -39,6 +39,8 @@ public interface ThreadLocalChangeListener {
      */
     
     void changed(Mode mode, Thread thread, ThreadLocal<?> threadLocal, Object value);
+    
+    boolean isEnabled();
 
     enum Mode {
         ADDED, REMOVED
diff --git a/src/main/java/org/apache/sling/commons/threads/impl/ThreadLocalCleaner.java b/src/main/java/org/apache/sling/commons/threads/impl/ThreadLocalCleaner.java
index 3ac4b52..5db69ec 100644
--- a/src/main/java/org/apache/sling/commons/threads/impl/ThreadLocalCleaner.java
+++ b/src/main/java/org/apache/sling/commons/threads/impl/ThreadLocalCleaner.java
@@ -145,8 +145,10 @@ public class ThreadLocalCleaner {
 
     public void cleanup() {
         // the first two diff calls are only to notify the listener, the actual cleanup is done by restoreOldThreadLocals
-        diff(threadLocalsField, threadLocalsCopy.references);
-        diff(inheritableThreadLocalsField, inheritableThreadLocalsCopy.references);
+        if ( listener.isEnabled() ) {
+            diff(threadLocalsField, threadLocalsCopy.references);
+            diff(inheritableThreadLocalsField, inheritableThreadLocalsCopy.references);
+        }
         restoreOldThreadLocals();
     }
 
diff --git a/src/test/java/org/apache/sling/commons/threads/impl/ThreadPoolExecutorCleaningThreadLocalsTest.java b/src/test/java/org/apache/sling/commons/threads/impl/ThreadPoolExecutorCleaningThreadLocalsTest.java
index 0160e64..8ceff1f 100644
--- a/src/test/java/org/apache/sling/commons/threads/impl/ThreadPoolExecutorCleaningThreadLocalsTest.java
+++ b/src/test/java/org/apache/sling/commons/threads/impl/ThreadPoolExecutorCleaningThreadLocalsTest.java
@@ -54,6 +54,7 @@ public class ThreadPoolExecutorCleaningThreadLocalsTest {
         pool = new ThreadPoolExecutorCleaningThreadLocals(
                     1, 1, 100, TimeUnit.MILLISECONDS,
                     queue, Executors.defaultThreadFactory(), rejectionHandler, listener);
+        Mockito.when(listener.isEnabled()).thenReturn(true);
     }
     
     @Test(timeout = 10000)

-- 
To stop receiving notification emails like this one, please contact
rombert@apache.org.