diff --git a/lib/src/main/java/com/cloudbees/groovy/cps/Continuable.java b/lib/src/main/java/com/cloudbees/groovy/cps/Continuable.java index 88988cd2f..ccecd04f1 100644 --- a/lib/src/main/java/com/cloudbees/groovy/cps/Continuable.java +++ b/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 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 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; } /** diff --git a/lib/src/test/java/com/cloudbees/groovy/cps/CpsDefaultGroovyMethodsTest.java b/lib/src/test/java/com/cloudbees/groovy/cps/CpsDefaultGroovyMethodsTest.java index 44916cb9e..3e37d4546 100644 --- a/lib/src/test/java/com/cloudbees/groovy/cps/CpsDefaultGroovyMethodsTest.java +++ b/lib/src/test/java/com/cloudbees/groovy/cps/CpsDefaultGroovyMethodsTest.java @@ -334,8 +334,6 @@ public static Iterable 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 diff --git a/lib/src/test/java/com/cloudbees/groovy/cps/CpsTransformerTest.java b/lib/src/test/java/com/cloudbees/groovy/cps/CpsTransformerTest.java index b28f707e9..481fd5d43 100644 --- a/lib/src/test/java/com/cloudbees/groovy/cps/CpsTransformerTest.java +++ b/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" + diff --git a/lib/src/test/java/com/cloudbees/groovy/cps/impl/NotBlockTest.java b/lib/src/test/java/com/cloudbees/groovy/cps/impl/NotBlockTest.java index e52d040d3..4fa5247c1 100644 --- a/lib/src/test/java/com/cloudbees/groovy/cps/impl/NotBlockTest.java +++ b/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)); } } diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsThread.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsThread.java index 1bdaa54fe..aa8512c36 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsThread.java +++ b/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 CATEGORIES = ImmutableList.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 { diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsVmExecutorService.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsVmExecutorService.java index 289176eda..9a67b0fa2 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsVmExecutorService.java +++ b/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 CATEGORIES = ImmutableList.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(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 Callable wrap(final Callable 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); } }; } diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/SandboxContinuable.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/SandboxContinuable.java index 12f06e6ca..64126326c 100644 --- a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/SandboxContinuable.java +++ b/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 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 categories) { trustedShell.getClassLoader(), shell.getClassLoader())); try (GroovySandbox.Scope scope = sandbox.enter()) { - return SandboxContinuable.super.run0(cn, categories); + return SandboxContinuable.super.run0(cn); } } diff --git a/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecutionMemoryTest.java b/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecutionMemoryTest.java index 31fe68d95..26d286930 100644 --- a/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecutionMemoryTest.java +++ b/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));