You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2023/05/11 08:20:41 UTC

[tomcat] branch 9.0.x updated: Use the container executor if available

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

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


The following commit(s) were added to refs/heads/9.0.x by this push:
     new 253641c728 Use the container executor if available
253641c728 is described below

commit 253641c7287ee1f906bad562836931413083a440
Author: remm <re...@apache.org>
AuthorDate: Thu May 11 10:15:28 2023 +0200

    Use the container executor if available
    
    As promised in the PR.
---
 .../apache/catalina/filters/RateLimitFilter.java   |  9 ++-
 .../apache/catalina/util/LocalStrings.properties   |  2 +
 .../apache/catalina/util/TimeBucketCounter.java    | 89 ++++++++++++++--------
 .../catalina/util/TestTimeBucketCounter.java       |  4 +-
 4 files changed, 68 insertions(+), 36 deletions(-)

diff --git a/java/org/apache/catalina/filters/RateLimitFilter.java b/java/org/apache/catalina/filters/RateLimitFilter.java
index d164eee94e..887a551273 100644
--- a/java/org/apache/catalina/filters/RateLimitFilter.java
+++ b/java/org/apache/catalina/filters/RateLimitFilter.java
@@ -18,6 +18,7 @@
 package org.apache.catalina.filters;
 
 import java.io.IOException;
+import java.util.concurrent.ScheduledExecutorService;
 
 import javax.servlet.FilterChain;
 import javax.servlet.FilterConfig;
@@ -31,6 +32,7 @@ import org.apache.catalina.util.TimeBucketCounter;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
 import org.apache.tomcat.util.res.StringManager;
+import org.apache.tomcat.util.threads.ScheduledThreadPoolExecutor;
 
 /**
  * <p>
@@ -191,7 +193,12 @@ public class RateLimitFilter extends GenericFilter {
             statusMessage = param;
         }
 
-        bucketCounter = new TimeBucketCounter(bucketDuration);
+        ScheduledExecutorService executorService =
+                (ScheduledExecutorService) getServletContext().getAttribute(ScheduledThreadPoolExecutor.class.getName());
+        if (executorService == null) {
+            executorService = new java.util.concurrent.ScheduledThreadPoolExecutor(1);
+        }
+        bucketCounter = new TimeBucketCounter(bucketDuration, executorService);
 
         actualRequests = (int) Math.round(bucketCounter.getRatio() * bucketRequests);
 
diff --git a/java/org/apache/catalina/util/LocalStrings.properties b/java/org/apache/catalina/util/LocalStrings.properties
index fe0e7875ce..d1d5bcffab 100644
--- a/java/org/apache/catalina/util/LocalStrings.properties
+++ b/java/org/apache/catalina/util/LocalStrings.properties
@@ -53,3 +53,5 @@ sessionIdGeneratorBase.noSHA1PRNG=The default SHA1PRNG algorithm for SecureRando
 sessionIdGeneratorBase.random=Exception initializing random number generator of class [{0}]. Falling back to java.secure.SecureRandom
 sessionIdGeneratorBase.randomAlgorithm=Exception initializing random number generator using algorithm [{0}]
 sessionIdGeneratorBase.randomProvider=Exception initializing random number generator using provider [{0}]
+
+timebucket.maintenance.error=Error processing periodic maintenance
diff --git a/java/org/apache/catalina/util/TimeBucketCounter.java b/java/org/apache/catalina/util/TimeBucketCounter.java
index 9a472523c7..e106e24e76 100644
--- a/java/org/apache/catalina/util/TimeBucketCounter.java
+++ b/java/org/apache/catalina/util/TimeBucketCounter.java
@@ -18,8 +18,16 @@
 package org.apache.catalina.util;
 
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.res.StringManager;
+
 /**
  * This class maintains a thread safe hash map that has timestamp-based buckets followed by a string for a key, and a
  * counter for a value. each time the increment() method is called it adds the key if it does not exist, increments its
@@ -27,6 +35,9 @@ import java.util.concurrent.atomic.AtomicInteger;
  */
 public class TimeBucketCounter {
 
+    private static final Log log = LogFactory.getLog(TimeBucketCounter.class);
+    private static final StringManager sm = StringManager.getManager(TimeBucketCounter.class);
+
     /**
      * Map to hold the buckets
      */
@@ -43,16 +54,22 @@ public class TimeBucketCounter {
     private final double ratio;
 
     /**
-     * Flag for the maintenance thread
+     * The future allowing control of the background processor.
      */
-    volatile boolean isRunning = false;
+    private ScheduledFuture<?> maintenanceFuture;
+    private ScheduledFuture<?> monitorFuture;
+    private final ScheduledExecutorService executorService;
+    private final long sleeptime;
 
     /**
      * Creates a new TimeBucketCounter with the specified lifetime.
      *
      * @param bucketDuration duration in seconds, e.g. for 1 minute pass 60
+     * @param executorService the executor service which will be used to run the maintenance
      */
-    public TimeBucketCounter(int bucketDuration) {
+    public TimeBucketCounter(int bucketDuration, ScheduledExecutorService executorService) {
+
+        this.executorService = executorService;
 
         int durationMillis = bucketDuration * 1000;
 
@@ -68,8 +85,13 @@ public class TimeBucketCounter {
         this.ratio = ratioToPowerOf2(durationMillis);
 
         int cleanupsPerBucketDuration = (durationMillis >= 60_000) ? 6 : 3;
-        Thread mt = new MaintenanceThread(durationMillis / cleanupsPerBucketDuration);
-        mt.start();
+        sleeptime = durationMillis / cleanupsPerBucketDuration;
+
+        // Start our thread
+        if (sleeptime > 0) {
+            monitorFuture = executorService
+                    .scheduleWithFixedDelay(new MaintenanceMonitor(), 0, 60, TimeUnit.SECONDS);
+        }
     }
 
     /**
@@ -156,43 +178,44 @@ public class TimeBucketCounter {
      * Sets isRunning to false to terminate the maintenance thread.
      */
     public void destroy() {
-        this.isRunning = false;
-    }
-
-    /**
-     * This class runs a background thread to clean up old keys from the map.
-     */
-    class MaintenanceThread extends Thread {
-
-        final long sleeptime;
-
-        MaintenanceThread(long sleeptime) {
-            super.setDaemon(true);
-            this.sleeptime = sleeptime;
+        // Stop our thread
+        if (monitorFuture != null) {
+            monitorFuture.cancel(true);
+            monitorFuture = null;
+        }
+        if (maintenanceFuture != null) {
+            maintenanceFuture.cancel(true);
+            maintenanceFuture = null;
         }
+    }
 
-        @SuppressWarnings("sync-override")
+    private class Maintenance implements Runnable {
         @Override
-        public void start() {
-            isRunning = true;
-            super.start();
+        public void run() {
+            String currentBucketPrefix = String.valueOf(getCurrentBucketPrefix());
+            ConcurrentHashMap.KeySetView<String,AtomicInteger> keys = map.keySet();
+            // remove obsolete keys
+            keys.removeIf(k -> !k.startsWith(currentBucketPrefix));
         }
+    }
 
+    private class MaintenanceMonitor implements Runnable {
         @Override
         public void run() {
-
-            while (isRunning) {
-                String currentBucketPrefix = String.valueOf(getCurrentBucketPrefix());
-                ConcurrentHashMap.KeySetView<String,AtomicInteger> keys = map.keySet();
-
-                // remove obsolete keys
-                keys.removeIf(k -> !k.startsWith(currentBucketPrefix));
-
-                try {
-                    Thread.sleep(sleeptime);
-                } catch (InterruptedException e) {
+            if (sleeptime > 0 &&
+                    (maintenanceFuture == null || maintenanceFuture.isDone())) {
+                if (maintenanceFuture != null && maintenanceFuture.isDone()) {
+                    // There was an error executing the scheduled task, get it and log it
+                    try {
+                        maintenanceFuture.get();
+                    } catch (InterruptedException | ExecutionException e) {
+                        log.error(sm.getString("timebucket.maintenance.error"), e);
+                    }
                 }
+                maintenanceFuture = executorService.scheduleWithFixedDelay(new Maintenance(), sleeptime, sleeptime,
+                        TimeUnit.MILLISECONDS);
             }
         }
     }
+
 }
diff --git a/test/org/apache/catalina/util/TestTimeBucketCounter.java b/test/org/apache/catalina/util/TestTimeBucketCounter.java
index b1a7fd8606..43f505eb43 100644
--- a/test/org/apache/catalina/util/TestTimeBucketCounter.java
+++ b/test/org/apache/catalina/util/TestTimeBucketCounter.java
@@ -44,7 +44,7 @@ public class TestTimeBucketCounter {
 
     @Test
     public void testTimeBucketCounter() {
-        TimeBucketCounter tbc = new TimeBucketCounter(60);
+        TimeBucketCounter tbc = new TimeBucketCounter(60, new java.util.concurrent.ScheduledThreadPoolExecutor(1));
         Assert.assertEquals(16, tbc.getNumBits());
         Assert.assertEquals(1.092, tbc.getRatio(), DELTA);
     }
@@ -54,7 +54,7 @@ public class TestTimeBucketCounter {
         long millis;
         int tb1, tb2;
 
-        TimeBucketCounter tbc = new TimeBucketCounter(2);
+        TimeBucketCounter tbc = new TimeBucketCounter(2, new java.util.concurrent.ScheduledThreadPoolExecutor(1));
         tb1 = tbc.getCurrentBucketPrefix();
         millis = tbc.getMillisUntilNextBucket();
 


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