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 2021/09/21 11:53:25 UTC

[sling-org-apache-sling-commons-threads] 02/02: SLING-10835 - ThreadLocal cleaner fallback code does not work

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

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

commit 8aee21f9cf21f7fd44786ccadb54acf6ac4fe8e4
Author: Robert Munteanu <ro...@apache.org>
AuthorDate: Tue Sep 21 13:51:29 2021 +0200

    SLING-10835 - ThreadLocal cleaner fallback code does not work
    
    Try harder to validate that the ThreadLocalCleaner works when instantiating the ThreadPool. This
    way we don't fail when tasks are submitted, which breaks the ThreadPool.
    
    Also removed the checks for ThreadLocalCleaner not working when submitting tasks since we validate
    that it works in the ThreadPool constructor.
---
 .../apache/sling/commons/threads/impl/DefaultThreadPool.java  |  2 +-
 .../apache/sling/commons/threads/impl/ThreadLocalCleaner.java |  9 +++++++++
 .../threads/impl/ThreadPoolExecutorCleaningThreadLocals.java  | 11 +++--------
 3 files changed, 13 insertions(+), 9 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 e60cf97..0287eba 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
@@ -153,7 +153,7 @@ public class DefaultThreadPool
                     threadFactory,
                     handler,
                     new LoggingThreadLocalChangeListener());
-        } catch (IllegalStateException e) {
+        } catch (RuntimeException | Error e) {
             logger.warn("Unsupported JRE, cannot register ThreadPoolExecutorCleaningThreadLocals due to '{}', fall back to regular ThreadPoolExecutor", e.getMessage(), e);
             this.executor = new ThreadPoolExecutor(this.configuration.getMinPoolSize(),
                     this.configuration.getMaxPoolSize(),
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 5db69ec..7e024b8 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
@@ -67,6 +67,15 @@ public class ThreadLocalCleaner {
         }
     }
     
+    static void validate() {
+        LOG.info("Validating reflective access is permitted... ");
+        Reference<?>[] threadLocals = copy(threadLocalsField);
+        if ( threadLocals == null )
+            LOG.info("Accessed thread locals of current thread, found none");
+        else
+            LOG.info("Accessed thread locals of current thread, found {}", threadLocals.length);
+    }
+
     /** @param c the class containing the field
      * @param name the name of the field
      * @return the field from the given class with the given name (made accessible)
diff --git a/src/main/java/org/apache/sling/commons/threads/impl/ThreadPoolExecutorCleaningThreadLocals.java b/src/main/java/org/apache/sling/commons/threads/impl/ThreadPoolExecutorCleaningThreadLocals.java
index ca1fd3b..38a5e38 100644
--- a/src/main/java/org/apache/sling/commons/threads/impl/ThreadPoolExecutorCleaningThreadLocals.java
+++ b/src/main/java/org/apache/sling/commons/threads/impl/ThreadPoolExecutorCleaningThreadLocals.java
@@ -49,19 +49,14 @@ public class ThreadPoolExecutorCleaningThreadLocals extends ThreadPoolExecutor {
             ThreadLocalChangeListener listener) {
         super(corePoolSize, maximumPoolSize, keepAliveTime, unit, 
                 workQueue, threadFactory, handler);
+        ThreadLocalCleaner.validate();
         this.listener = listener;
     }
 
     protected void beforeExecute(Thread t, Runnable r) {
         LOGGER.debug("Collecting changes to ThreadLocal for thread {} from now on...", t);
-        try {
-            ThreadLocalCleaner cleaner = new ThreadLocalCleaner(listener);
-            cleaners.put(t, cleaner);
-        } catch (RuntimeException | Error e) {
-            LOGGER.warn("Could not set up thread local cleaner (most probably not a compliant JRE): {}", e, e);
-            throw e;
-        }
-        
+        ThreadLocalCleaner cleaner = new ThreadLocalCleaner(listener);
+        cleaners.put(t, cleaner);
         super.beforeExecute(t, r);
     }