Merge pull request #495 from Dohbedoh/JENKINS-74975

[JENKINS-74975] Fix Semaphore over-allocation of permits / limit all CleanupTasks

Author: dwnusbaum

src/main/java/jenkins/branch/WorkspaceLocatorImpl.java

@@ -43,6 +43,10 @@
 import hudson.security.ACLContext;
 import hudson.slaves.ComputerListener;
 import hudson.slaves.WorkspaceList;
+import hudson.util.ClassLoaderSanityThreadFactory;
+import hudson.util.DaemonThreadFactory;
+import hudson.util.ExceptionCatchingThreadFactory;
+import hudson.util.NamingThreadFactory;
 import hudson.util.TextFile;
 import java.io.BufferedReader;
 import java.io.File;
@@ -53,23 +57,31 @@
 import java.nio.charset.StandardCharsets;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
-import java.util.concurrent.Semaphore;
+import java.util.concurrent.ExecutorService;
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Queue;
 import java.util.TreeMap;
 import java.util.WeakHashMap;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.SynchronousQueue;
+import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import jenkins.MasterToSlaveFileCallable;
 import jenkins.model.Jenkins;
+import jenkins.security.ImpersonatingExecutorService;
 import jenkins.slaves.WorkspaceLocator;
+import jenkins.util.ContextResettingExecutorService;
+import jenkins.util.ErrorLoggingExecutorService;
 import jenkins.util.SystemProperties;
 import org.apache.commons.codec.binary.Base32;
 import org.apache.commons.lang.StringUtils;
@@ -380,24 +392,49 @@ static String minimize(String name) {
     @Extension
     public static class Deleter extends ItemListener {
 
-        private static /* almost final */ int CLEANUP_THREAD_LIMIT = SystemProperties.getInteger(Deleter.class.getName() + ".CLEANUP_THREAD_LIMIT", Integer.valueOf(0)).intValue();
+        private static final int CLEANUP_THREAD_LIMIT = SystemProperties.getInteger(Deleter.class.getName() + ".CLEANUP_THREAD_LIMIT", 0);
 
-        /** Semaphore for limiting number of scheduled {@link CleanupTask} */
-        private static Semaphore cleanupPool = new Semaphore(CLEANUP_THREAD_LIMIT, true);
+        private static final ExecutorService executorService = executorService();
 
         /** Number of {@link CleanupTask} which have been scheduled but not yet completed. */
         private static int runningTasks;
 
+        private static ExecutorService executorService() {
+            if (CLEANUP_THREAD_LIMIT > 0) {
+                ThreadPoolExecutor tpe = new ThreadPoolExecutor(CLEANUP_THREAD_LIMIT, CLEANUP_THREAD_LIMIT, 10L, TimeUnit.SECONDS, new LinkedBlockingQueue<>(),
+                    new ExceptionCatchingThreadFactory(
+                        new NamingThreadFactory(
+                            new ClassLoaderSanityThreadFactory(new DaemonThreadFactory()),
+                            "Deleter.cleanupTask")));
+                // Use allowCoreThreadTimeOut to keep a lightweight ThreadPoolExecutor. The Thread pool grows to the
+                // limit and then queue tasks. Thread will then be removed when idle
+                tpe.allowCoreThreadTimeOut(true);
+                return new ContextResettingExecutorService(
+                    new ImpersonatingExecutorService(
+                        new ErrorLoggingExecutorService(tpe),
+                            ACL.SYSTEM2));
+            } else {
+                return Computer.threadPoolForRemoting;
+            }
+        }
+
         @Override
         public void onDeleted(Item item) {
             if (!(item instanceof TopLevelItem)) {
                 return;
             }
             TopLevelItem tli = (TopLevelItem) item;
             Jenkins jenkins = Jenkins.get();
-            Computer.threadPoolForRemoting.submit(new CleanupTask(tli, jenkins));
-            // Starts provisioner Thread which is tasked with starting cleanup Threads
-            new CleanupTaskProvisioner(tli, jenkins.getNodes()).run();
+            Queue<Node> nodes = Stream
+                .concat(Stream.of(jenkins), jenkins.getNodes().stream())
+                .collect(Collectors.toCollection(LinkedList::new));
+            try {
+                while (!nodes.isEmpty()){
+                    executorService.execute(new CleanupTask(tli, nodes.remove()));
+                }
+            } catch (Exception e) {
+                LOGGER.log(Level.WARNING, e.getMessage());
+            }
         }
 
         @Override
@@ -412,20 +449,6 @@ public void onLocationChanged(Item item, String oldFullName, String newFullName)
             }
         }
 
-        public void acquireThread() throws InterruptedException {
-            if (CLEANUP_THREAD_LIMIT <= 0) {
-                return;
-            }
-            cleanupPool.acquire();
-        }
-
-        public void releaseThread() {
-            if (CLEANUP_THREAD_LIMIT <= 0) {
-                return;
-            }
-            cleanupPool.release();
-        }
-
         // Visible for testing
         static synchronized void waitForTasksToFinish() throws InterruptedException {
             while (runningTasks > 0) {
@@ -442,36 +465,6 @@ private static synchronized void taskFinished() {
             Deleter.class.notifyAll();
         }
 
-        private static class CleanupTaskProvisioner implements Runnable{
-
-            @NonNull
-            private final TopLevelItem tli;
-
-            @NonNull
-            private final Queue<Node> nodes;
-
-            @NonNull
-            private final Deleter deleter;
-
-            public CleanupTaskProvisioner(TopLevelItem tli, List<Node> nodes) {
-                this.tli = tli;
-                this.nodes = new LinkedList<>(nodes);
-                this.deleter = ExtensionList.lookupSingleton(Deleter.class);
-            }
-
-            @Override
-            public void run() {
-                try {
-                    while (!nodes.isEmpty()){
-                        deleter.acquireThread();
-                        Computer.threadPoolForRemoting.submit(new CleanupTask(tli, nodes.remove()));
-                    }
-                } catch (Exception e) {
-                    LOGGER.log(Level.WARNING, e.getMessage());
-                }
-            }
-        }
-
         private static class CleanupTask implements Runnable {
 
             @NonNull
@@ -480,13 +473,9 @@ private static class CleanupTask implements Runnable {
             @NonNull
             private final Node node;
 
-            @NonNull
-            private final Deleter deleter;
-
             CleanupTask(TopLevelItem tli, Node node) {
                 this.tli = tli;
                 this.node = node;
-                this.deleter = ExtensionList.lookupSingleton(Deleter.class);
                 taskStarted();
             }
 
@@ -529,7 +518,6 @@ public void run() {
                     }
                 } finally {
                     t.setName(oldName);
-                    deleter.releaseThread();
                     taskFinished();
                 }
             }