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 12:05:36 UTC

[sling-org-apache-sling-commons-threads] branch master updated (d7e6e9f -> 84591a3)

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

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


    from d7e6e9f  SLING-10676 - remove SECURITY.md which is not needed
     new 716c2cf  SLING-10835 - ThreadLocal cleaner fallback code does not work
     new 84591a3  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.


Summary of changes:
 .../apache/sling/commons/threads/impl/DefaultThreadPool.java  |  2 +-
 .../apache/sling/commons/threads/impl/ThreadLocalCleaner.java |  9 +++++++++
 .../threads/impl/ThreadPoolExecutorCleaningThreadLocals.java  | 11 +++--------
 .../impl/ThreadPoolExecutorCleaningThreadLocalsTest.java      |  2 +-
 4 files changed, 14 insertions(+), 10 deletions(-)

[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 master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-commons-threads.git

commit 716c2cf55d06aea2838bb6dfafc11bd46cd3537c
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 master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-commons-threads.git

commit 84591a3ff805feac3e2f80df542ca3afcb5935ef
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);
     }