Merge pull request jenkinsci#877 from jglick/GroovyCategorySupport

More efficient use of `GroovyCategorySupport`

lib/src/main/java/com/cloudbees/groovy/cps/Continuable.java

@@ -23,6 +23,9 @@
  */
 public class Continuable implements Serializable {
 
+    /**
+     * Users of this library must pass (at least) these to {@link GroovyCategorySupport#use(List, Closure)} during all operations.
+     */
     @SuppressWarnings("rawtypes")
     public static final List<Class> categories = List.of(
         CpsDefaultGroovyMethods.class,
@@ -133,31 +136,21 @@ public Object runByThrow(Throwable arg) throws InvocationTargetException {
         return run0(new Outcome(null,arg)).wrapReplay();
     }
 
-    @Deprecated
-    public Outcome run0(final Outcome cn) {
-        return run0(cn, categories);
-    }
-
     /**
      * Resumes this program by either returning the value from {@link Continuable#suspend(Object)} or
      * throwing an exception
      */
-    public Outcome run0(final Outcome cn, List<Class> categories) {
-        return GroovyCategorySupport.use(categories, new Closure<>(null) {
-            @Override
-            public Outcome call() {
-                Next n = cn.resumeFrom(e,k);
-
-                while(n.yield==null) {
-                    n = n.step();
-                }
-
-                e = n.e;
-                k = n.k;
-
-                return n.yield;
-            }
-        });
+    public Outcome run0(final Outcome cn) {
+        Next n = cn.resumeFrom(e,k);
+
+        while(n.yield==null) {
+            n = n.step();
+        }
+
+        e = n.e;
+        k = n.k;
+
+        return n.yield;
     }
 
     /**

lib/src/test/java/com/cloudbees/groovy/cps/CpsDefaultGroovyMethodsTest.java

@@ -334,8 +334,6 @@ public static Iterable<Object[]> generateParameters() {
             asList("uniqueSet", "([1, 2, -2, 3] as HashSet).unique { i -> i * i }.collect { it.abs() } as HashSet", asList(1, 2, 3) as HashSet),
             */
 
-            // TODO: use?
-
             // TODO: with?
 
             // .withDefault

lib/src/test/java/com/cloudbees/groovy/cps/CpsTransformerTest.java

@@ -769,7 +769,7 @@ public void rehydrateClosure() throws Throwable {
 
     @Issue("https://github.com/cloudbees/groovy-cps/issues/16")
     @Test
-    @Ignore
+    @Ignore("cannot easily be supported")
     public void category() throws Throwable {
         assertEvaluate("FOO",
             "class BarCategory {\n" +

lib/src/test/java/com/cloudbees/groovy/cps/impl/NotBlockTest.java

@@ -7,7 +7,6 @@
 import groovy.lang.Script;
 import java.io.InputStream;
 import java.io.ObjectInputStream;
-import java.util.Collections;
 import org.junit.Test;
 
 import static org.hamcrest.CoreMatchers.equalTo;
@@ -38,6 +37,6 @@ public void serialFormBackwardsCompatibility() throws Throwable {
             c = (Continuable)ois.readObject();
         }
         assertTrue(c.isResumable());
-        assertThat(c.run0(new Outcome(false, null), Collections.emptyList()).replay(), equalTo(true));
+        assertThat(c.run0(new Outcome(false, null)).replay(), equalTo(true));
     }
 }

plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsThread.java

@@ -26,7 +26,6 @@
 
 import com.cloudbees.groovy.cps.Continuable;
 import com.cloudbees.groovy.cps.Outcome;
-import com.google.common.collect.ImmutableList;
 import com.google.common.util.concurrent.FutureCallback;
 import java.io.IOException;
 import org.jenkinsci.plugins.workflow.cps.persistence.PersistIn;
@@ -43,7 +42,6 @@
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Logger;
 
-import org.jenkinsci.plugins.workflow.cps.persistence.IteratorHack;
 
 import static java.util.logging.Level.FINE;
 import static org.jenkinsci.plugins.workflow.cps.persistence.PersistenceContext.PROGRAM;
@@ -162,11 +160,6 @@ public StepExecution getStep() {
         this.step = step;
     }
 
-    private static final List<Class> CATEGORIES = ImmutableList.<Class>builder()
-            .addAll(Continuable.categories)
-            .add(IteratorHack.class)
-            .build();
-
     /**
      * Executes CPS code synchronously a little bit more, until it hits
      * the point the workflow needs to be dehydrated.
@@ -184,7 +177,7 @@ public StepExecution getStep() {
             LOGGER.fine(() -> "runNextChunk on " + resumeValue);
             final Outcome o = resumeValue;
             resumeValue = null;
-            outcome = program.run0(o, CATEGORIES);
+            outcome = program.run0(o);
             if (outcome.getAbnormal() != null) {
                 LOGGER.log(FINE, "ran and produced error", outcome.getAbnormal());
             } else {

plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsVmExecutorService.java

@@ -1,8 +1,9 @@
 package org.jenkinsci.plugins.workflow.cps;
 
+import com.cloudbees.groovy.cps.Continuable;
 import com.cloudbees.groovy.cps.impl.CpsCallableInvocation;
+import com.google.common.collect.ImmutableList;
 import hudson.Main;
-import hudson.model.Computer;
 import hudson.remoting.SingleLaneExecutorService;
 import hudson.security.ACL;
 import java.io.IOException;
@@ -11,9 +12,21 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import edu.umd.cs.findbugs.annotations.CheckForNull;
+import groovy.lang.Closure;
 import hudson.model.TaskListener;
+import hudson.util.DaemonThreadFactory;
+import hudson.util.ExceptionCatchingThreadFactory;
+import hudson.util.NamingThreadFactory;
+import java.util.List;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ThreadFactory;
 import jenkins.model.Jenkins;
+import jenkins.security.ImpersonatingExecutorService;
+import jenkins.util.ContextResettingExecutorService;
+import jenkins.util.ErrorLoggingExecutorService;
 import jenkins.util.InterceptingExecutorService;
+import org.codehaus.groovy.runtime.GroovyCategorySupport;
+import org.jenkinsci.plugins.workflow.cps.persistence.IteratorHack;
 import org.jenkinsci.plugins.workflow.graph.FlowNode;
 import org.jenkinsci.plugins.workflow.steps.StepExecution;
 
@@ -24,27 +37,55 @@
  * @see CpsVmThreadOnly
  */
 class CpsVmExecutorService extends InterceptingExecutorService {
+
+    @SuppressWarnings("rawtypes")
+    private static final List<Class> CATEGORIES = ImmutableList.<Class>builder()
+        .addAll(Continuable.categories)
+        .add(IteratorHack.class)
+        .build();
+
+    private static ThreadFactory categoryThreadFactory(ThreadFactory core) {
+        return r -> core.newThread(() -> {
+            LOGGER.fine("spawning new thread");
+            GroovyCategorySupport.use(CATEGORIES, new Closure<Void>(null) {
+                @Override
+                public Void call() {
+                    r.run();
+                    return null;
+                }
+            });
+        });
+    }
+
+    private static final ExecutorService threadPool = new ContextResettingExecutorService(
+        new ImpersonatingExecutorService(
+            new ErrorLoggingExecutorService(
+                Executors.newCachedThreadPool(
+                    categoryThreadFactory(
+                        new ExceptionCatchingThreadFactory(
+                            new NamingThreadFactory(
+                                new DaemonThreadFactory(),
+                                "CpsVmExecutorService"))))),
+            ACL.SYSTEM2));
+
     private CpsThreadGroup cpsThreadGroup;
 
-    public CpsVmExecutorService(CpsThreadGroup cpsThreadGroup) {
-        super(new SingleLaneExecutorService(Computer.threadPoolForRemoting));
+    CpsVmExecutorService(CpsThreadGroup cpsThreadGroup) {
+        super(new SingleLaneExecutorService(threadPool));
         this.cpsThreadGroup = cpsThreadGroup;
     }
 
     @Override
     protected Runnable wrap(final Runnable r) {
-        return new Runnable() {
-            @Override
-            public void run() {
-                ThreadContext context = setUp();
-                try {
-                    r.run();
-                } catch (final Throwable t) {
-                    reportProblem(t);
-                    throw t;
-                } finally {
-                    tearDown(context);
-                }
+        return () -> {
+            ThreadContext context = setUp();
+            try {
+                r.run();
+            } catch (final Throwable t) {
+                reportProblem(t);
+                throw t;
+            } finally {
+                tearDown(context);
             }
         };
     }
@@ -89,18 +130,15 @@ private void reportProblem(Throwable t) {
 
     @Override
     protected <V> Callable<V> wrap(final Callable<V> r) {
-        return new Callable<>() {
-            @Override
-            public V call() throws Exception {
-                ThreadContext context = setUp();
-                try {
-                    return r.call();
-                } catch (final Throwable t) {
-                    reportProblem(t);
-                    throw t;
-                } finally {
-                    tearDown(context);
-                }
+        return () -> {
+            ThreadContext context = setUp();
+            try {
+                return r.call();
+            } catch (final Throwable t) {
+                reportProblem(t);
+                throw t;
+            } finally {
+                tearDown(context);
             }
         };
     }

plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/SandboxContinuable.java

@@ -7,7 +7,6 @@
 import hudson.console.ConsoleAnnotator;
 import hudson.console.ConsoleNote;
 import java.io.IOException;
-import java.util.List;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.GroovySandbox;
@@ -24,8 +23,7 @@ class SandboxContinuable extends Continuable {
     }
 
     @SuppressWarnings("rawtypes")
-    @Override
-    public Outcome run0(final Outcome cn, final List<Class> categories) {
+    public Outcome run0(final Outcome cn) {
         CpsFlowExecution e = thread.group.getExecution();
         if (e == null) {
             throw new IllegalStateException("JENKINS-50407: no loaded execution");
@@ -48,7 +46,7 @@ public Outcome run0(final Outcome cn, final List<Class> categories) {
             trustedShell.getClassLoader(),
             shell.getClassLoader()));
         try (GroovySandbox.Scope scope = sandbox.enter()) {
-            return SandboxContinuable.super.run0(cn, categories);
+            return SandboxContinuable.super.run0(cn);
         }
     }
 

plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecutionMemoryTest.java

@@ -25,6 +25,7 @@
 package org.jenkinsci.plugins.workflow.cps;
 
 import groovy.lang.MetaClass;
+import hudson.model.Computer;
 import java.lang.ref.WeakReference;
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
@@ -67,6 +68,7 @@ public static void register(Object o) {
     }
 
     @Test public void loaderReleased() throws Exception {
+        Computer.threadPoolForRemoting.submit(() -> {}).get(); // do not let it be initialized inside the CpsVmExecutorService thread group
         WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
         r.jenkins.getWorkspaceFor(p).child("lib.groovy").write(CpsFlowExecutionMemoryTest.class.getName() + ".register(this)", null);
         p.setDefinition(new CpsFlowDefinition(CpsFlowExecutionMemoryTest.class.getName() + ".register(this); node {load 'lib.groovy'; evaluate(readFile('lib.groovy'))}", false));