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:23 UTC

[sling-org-apache-sling-commons-threads] branch issue/SLING-10835 created (now 8aee21f)

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

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


      at 8aee21f  SLING-10835 - ThreadLocal cleaner fallback code does not work

This branch includes the following new commits:

     new 872fd91  SLING-10835 - ThreadLocal cleaner fallback code does not work
     new 8aee21f  SLING-10835 - ThreadLocal cleaner fallback code does not work

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


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

Posted by ro...@apache.org.
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 872fd91eb3d7f24b6c71ea7c3a620a1346418e48
Author: Robert Munteanu <ro...@apache.org>
AuthorDate: Tue Sep 21 13:18:28 2021 +0200

    SLING-10835 - ThreadLocal cleaner fallback code does not work
    
    Set timeout for all test methods in ThreadPoolExecutorCleaningThreadLocalsTest,
    otherwise they hang indefinitely on Java 17.
---
 .../threads/impl/ThreadPoolExecutorCleaningThreadLocalsTest.java        | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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 bbaee70..a720d2b 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
@@ -69,7 +69,7 @@ public class ThreadPoolExecutorCleaningThreadLocalsTest {
         }
     }
 
-    @Test
+    @Test(timeout = 10000)
     public void testThreadLocalBeingCleanedUp() throws InterruptedException, ExecutionException {
         assertTaskDoesNotSeeOldThreadLocals("test");
         assertTaskDoesNotSeeOldThreadLocals("test2");

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

Posted by ro...@apache.org.
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);
     }