[tomcat] 01/02: Improve locking of utility executor

[markt]

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

markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit ea9a80fe743fca86362a2184806941b655bd0088
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu May 4 11:41:57 2023 +0100

    Improve locking of utility executor
    
    Fixes some edge cases around calling setUtilityThreads() during stop
---
 java/org/apache/catalina/core/StandardServer.java | 51 +++++++++++++----------
 1 file changed, 29 insertions(+), 22 deletions(-)

diff --git a/java/org/apache/catalina/core/StandardServer.java 
b/java/org/apache/catalina/core/StandardServer.java
index 8db5d8f05a..f7ac2d2e61 100644
--- a/java/org/apache/catalina/core/StandardServer.java
+++ b/java/org/apache/catalina/core/StandardServer.java
@@ -429,27 +429,30 @@ public final class StandardServer extends 
LifecycleMBeanBase implements Server {
             return;
         }
         this.utilityThreads = utilityThreads;
-        if (oldUtilityThreads != utilityThreads && utilityExecutor != null) {
-            
reconfigureUtilityExecutor(getUtilityThreadsInternal(utilityThreads));
+        synchronized (utilityExecutorLock) {
+            if (oldUtilityThreads != utilityThreads && utilityExecutor != 
null) {
+                
reconfigureUtilityExecutor(getUtilityThreadsInternal(utilityThreads));
+            }
         }
     }
 
 
+    /*
+     * Callers must be holding utilityExecutorLock
+     */
     private void reconfigureUtilityExecutor(int threads) {
-        synchronized (utilityExecutorLock) {
-            // The ScheduledThreadPoolExecutor doesn't use MaximumPoolSize, 
only CorePoolSize is available
-            if (utilityExecutor != null) {
-                utilityExecutor.setCorePoolSize(threads);
-            } else {
-                ScheduledThreadPoolExecutor scheduledThreadPoolExecutor = new 
ScheduledThreadPoolExecutor(threads,
-                        new TaskThreadFactory("Catalina-utility-", 
utilityThreadsAsDaemon, Thread.MIN_PRIORITY));
-                scheduledThreadPoolExecutor.setKeepAliveTime(10, 
TimeUnit.SECONDS);
-                scheduledThreadPoolExecutor.setRemoveOnCancelPolicy(true);
-                
scheduledThreadPoolExecutor.setExecuteExistingDelayedTasksAfterShutdownPolicy(false);
-                utilityExecutor = scheduledThreadPoolExecutor;
-                utilityExecutorWrapper = new 
org.apache.tomcat.util.threads.ScheduledThreadPoolExecutor(
-                        utilityExecutor);
-            }
+        // The ScheduledThreadPoolExecutor doesn't use MaximumPoolSize, only 
CorePoolSize is available
+        if (utilityExecutor != null) {
+            utilityExecutor.setCorePoolSize(threads);
+        } else {
+            ScheduledThreadPoolExecutor scheduledThreadPoolExecutor = new 
ScheduledThreadPoolExecutor(threads,
+                    new TaskThreadFactory("Catalina-utility-", 
utilityThreadsAsDaemon, Thread.MIN_PRIORITY));
+            scheduledThreadPoolExecutor.setKeepAliveTime(10, TimeUnit.SECONDS);
+            scheduledThreadPoolExecutor.setRemoveOnCancelPolicy(true);
+            
scheduledThreadPoolExecutor.setExecuteExistingDelayedTasksAfterShutdownPolicy(false);
+            utilityExecutor = scheduledThreadPoolExecutor;
+            utilityExecutorWrapper = new 
org.apache.tomcat.util.threads.ScheduledThreadPoolExecutor(
+                    utilityExecutor);
         }
     }
 
@@ -981,8 +984,10 @@ public final class StandardServer extends 
LifecycleMBeanBase implements Server {
         super.initInternal();
 
         // Initialize utility executor
-        reconfigureUtilityExecutor(getUtilityThreadsInternal(utilityThreads));
-        register(utilityExecutor, "type=UtilityExecutor");
+        synchronized (utilityExecutorLock) {
+            
reconfigureUtilityExecutor(getUtilityThreadsInternal(utilityThreads));
+            register(utilityExecutor, "type=UtilityExecutor");
+        }
 
         // Register global String cache
         // Note although the cache is global, if there are multiple Servers
@@ -1042,10 +1047,12 @@ public final class StandardServer extends 
LifecycleMBeanBase implements Server {
 
         unregister(onameStringCache);
 
-        if (utilityExecutor != null) {
-            utilityExecutor.shutdownNow();
-            unregister("type=UtilityExecutor");
-            utilityExecutor = null;
+        synchronized (utilityExecutorLock) {
+            if (utilityExecutor != null) {
+                utilityExecutor.shutdownNow();
+                unregister("type=UtilityExecutor");
+                utilityExecutor = null;
+            }
         }
 
         super.destroyInternal();


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org