From 6ba5df6459208ac38d831172761e6dc34155d7e9 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Sat, 16 Jan 2021 20:17:15 +0000 Subject: [PATCH 1/7] Upgrade guava --- .mvn/extensions.xml | 2 +- pom.xml | 15 +++++++--- .../concurrent/ChainingListenableFuture.java | 2 +- .../workflow/support/concurrent/Futures.java | 28 +++++++++---------- .../support/concurrent/ListFuture.java | 2 +- 5 files changed, 28 insertions(+), 21 deletions(-) diff --git a/.mvn/extensions.xml b/.mvn/extensions.xml index a2d496cc..43d62816 100644 --- a/.mvn/extensions.xml +++ b/.mvn/extensions.xml @@ -2,6 +2,6 @@ io.jenkins.tools.incrementals git-changelist-maven-extension - 1.0-beta-4 + 1.2 diff --git a/pom.xml b/pom.xml index de2bf95f..1215a977 100644 --- a/pom.xml +++ b/pom.xml @@ -44,9 +44,9 @@ - scm:git:git://github.com/jenkinsci/${project.artifactId}-plugin.git - scm:git:git@github.com:jenkinsci/${project.artifactId}-plugin.git - https://github.com/jenkinsci/${project.artifactId}-plugin + scm:git:git://github.com/${gitHubRepo}.git + scm:git:git@github.com:${gitHubRepo}.git + https://github.com/${gitHubRepo} ${scmTag} @@ -64,11 +64,12 @@ 3.8 -SNAPSHOT - 2.176.4 + 2.273-rc30689.09147672b85b 8 false true 3.0 + jenkinsci/${project.artifactId}-plugin @@ -97,6 +98,12 @@ org.jenkins-ci.plugins script-security + + + org.checkerframework + checker-qual + + diff --git a/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/ChainingListenableFuture.java b/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/ChainingListenableFuture.java index 1232d0fb..b1935ae0 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/ChainingListenableFuture.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/ChainingListenableFuture.java @@ -208,7 +208,7 @@ public void run() { ChainingListenableFuture.this.outputFuture = null; } } - }, MoreExecutors.sameThreadExecutor()); + }, MoreExecutors.newDirectExecutorService()); } catch (UndeclaredThrowableException e) { // Set the cause of the exception as this future's exception setException(e.getCause()); diff --git a/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/Futures.java b/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/Futures.java index 8900e6de..a882efd5 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/Futures.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/Futures.java @@ -65,7 +65,7 @@ public abstract class Futures { * });} * *

Note: This overload of {@code addCallback} is designed for cases in - * which the callack is fast and lightweight, as the method does not accept + * which the callback is fast and lightweight, as the method does not accept * an {@code Executor} in which to perform the the work. For heavier * callbacks, this overload carries some caveats: First, the thread that the * callback runs in depends on whether the input {@code Future} is done at the @@ -74,7 +74,7 @@ public abstract class Futures { * callback in the thread that calls {@code addCallback} or {@code * Future.cancel}. Second, callbacks may run in an internal thread of the * system responsible for the input {@code Future}, such as an RPC network - * thread. Finally, during the execution of a {@code sameThreadExecutor} + * thread. Finally, during the execution of a {@code newDirectExecutorService} * callback, all other registered but unexecuted listeners are prevented from * running, even if those listeners are to run in other executors. * @@ -87,7 +87,7 @@ public abstract class Futures { */ public static void addCallback(ListenableFuture future, FutureCallback callback) { - addCallback(future, callback, MoreExecutors.sameThreadExecutor()); + addCallback(future, callback, MoreExecutors.newDirectExecutorService()); } /** @@ -115,8 +115,8 @@ public static void addCallback(ListenableFuture future, * * When the callback is fast and lightweight consider {@linkplain * Futures#addCallback(ListenableFuture, FutureCallback) the other overload} - * or explicit use of {@link MoreExecutors#sameThreadExecutor - * sameThreadExecutor}. For heavier callbacks, this choice carries some + * or explicit use of {@link MoreExecutors#newDirectExecutorService + * newDirectExecutorService}. For heavier callbacks, this choice carries some * caveats: First, the thread that the callback runs in depends on whether * the input {@code Future} is done at the time {@code addCallback} is called * and on whether the input {@code Future} is ever cancelled. In particular, @@ -124,7 +124,7 @@ public static void addCallback(ListenableFuture future, * {@code addCallback} or {@code Future.cancel}. Second, callbacks may run in * an internal thread of the system responsible for the input {@code Future}, * such as an RPC network thread. Finally, during the execution of a {@code - * sameThreadExecutor} callback, all other registered but unexecuted + * newDirectExecutorService} callback, all other registered but unexecuted * listeners are prevented from running, even if those listeners are to run * in other executors. * @@ -218,7 +218,7 @@ public static ListenableFuture immediateFailedFuture( * thread that called {@code transform}. Second, transformations may run in * an internal thread of the system responsible for the input {@code Future}, * such as an RPC network thread. Finally, during the execution of a {@code - * sameThreadExecutor} transformation, all other registered but unexecuted + * newDirectExecutorService} transformation, all other registered but unexecuted * listeners are prevented from running, even if those listeners are to run * in other executors. * @@ -240,7 +240,7 @@ public static ListenableFuture immediateFailedFuture( */ public static ListenableFuture transform(ListenableFuture future, final Function function) { - return Futures.transform(future, function, MoreExecutors.sameThreadExecutor()); + return Futures.transform(future, function, MoreExecutors.newDirectExecutorService()); } /** @@ -272,7 +272,7 @@ public static ListenableFuture transform(ListenableFuture future, *

Note: For cases in which the transformation is fast and lightweight, * consider {@linkplain Futures#transform(ListenableFuture, Function) the * other overload} or explicit use of {@link - * MoreExecutors#sameThreadExecutor}. For heavier transformations, this + * MoreExecutors#newDirectExecutorService}. For heavier transformations, this * choice carries some caveats: First, the thread that the transformation * runs in depends on whether the input {@code Future} is done at the time * {@code transform} is called. In particular, if called late, {@code @@ -280,7 +280,7 @@ public static ListenableFuture transform(ListenableFuture future, * {@code transform}. Second, transformations may run in an internal thread * of the system responsible for the input {@code Future}, such as an RPC * network thread. Finally, during the execution of a {@code - * sameThreadExecutor} transformation, all other registered but unexecuted + * newDirectExecutorService} transformation, all other registered but unexecuted * listeners are prevented from running, even if those listeners are to run * in other executors. * @@ -324,7 +324,7 @@ public static ListenableFuture transform(ListenableFuture future, public static ListenableFuture> allAsList( ListenableFuture... futures) { return new ListFuture(ImmutableList.copyOf(futures), true, - MoreExecutors.sameThreadExecutor()); + MoreExecutors.newDirectExecutorService()); } /** @@ -347,7 +347,7 @@ public static ListenableFuture> allAsList( public static ListenableFuture> allAsList( Iterable> futures) { return new ListFuture(ImmutableList.copyOf(futures), true, - MoreExecutors.sameThreadExecutor()); + MoreExecutors.newDirectExecutorService()); } /** @@ -379,14 +379,14 @@ public static ListenableFuture> allAsList( *

Note: For cases in which the work of creating the derived future is * fast and lightweight, consider {@linkplain Futures#chain(ListenableFuture, * Function) the other overload} or explicit use of {@code - * sameThreadExecutor}. For heavier derivations, this choice carries some + * newDirectExecutorService}. For heavier derivations, this choice carries some * caveats: First, the thread that the derivation runs in depends on whether * the input {@code Future} is done at the time {@code chain} is called. In * particular, if called late, {@code chain} will run the derivation in the * thread that called {@code chain}. Second, derivations may run in an * internal thread of the system responsible for the input {@code Future}, * such as an RPC network thread. Finally, during the execution of a {@code - * sameThreadExecutor} {@code chain} function, all other registered but + * newDirectExecutorService} {@code chain} function, all other registered but * unexecuted listeners are prevented from running, even if those listeners * are to run in other executors. * diff --git a/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/ListFuture.java b/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/ListFuture.java index 61322fd1..44345679 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/ListFuture.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/ListFuture.java @@ -75,7 +75,7 @@ public void run() { // Let go of the memory held by other futures ListFuture.this.futures = null; } - }, MoreExecutors.sameThreadExecutor()); + }, MoreExecutors.newDirectExecutorService()); // Now begin the "real" initialization. From 3a320924f94d8d0cee6cb31face2617779c3fab9 Mon Sep 17 00:00:00 2001 From: Tim Jacomb <21194782+timja@users.noreply.github.com> Date: Thu, 4 Feb 2021 15:58:35 +0000 Subject: [PATCH 2/7] Update pom.xml --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 359da90c..bfa4d752 100644 --- a/pom.xml +++ b/pom.xml @@ -64,7 +64,7 @@ 3.8 -SNAPSHOT - 2.273-rc30689.09147672b85b + 2.273-rc30689.09147672b85b 8 false true From b0ce2e0afabe5b9ae4c6efea58e45367e70209c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Mu=C3=B1iz?= Date: Wed, 3 Mar 2021 17:31:34 +0100 Subject: [PATCH 3/7] Use reflection to fallback to older guava API and avoid core bump --- pom.xml | 7 ++- .../concurrent/ChainingListenableFuture.java | 2 +- .../workflow/support/concurrent/Futures.java | 44 +++++++++++++------ .../support/concurrent/ListFuture.java | 2 +- 4 files changed, 36 insertions(+), 19 deletions(-) diff --git a/pom.xml b/pom.xml index 1d855441..191367b3 100644 --- a/pom.xml +++ b/pom.xml @@ -64,7 +64,7 @@ 3.8 -SNAPSHOT - 2.273-rc30689.09147672b85b + 2.222.4 8 false true @@ -75,8 +75,8 @@ io.jenkins.tools.bom - bom-2.277.x - 23 + bom-2.222.x + 21 import pom @@ -90,7 +90,6 @@ org.jenkins-ci.plugins.workflow workflow-api - 2.42-rc1048.8d974b8c4ed5 org.jenkins-ci.plugins diff --git a/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/ChainingListenableFuture.java b/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/ChainingListenableFuture.java index b1935ae0..932a8c14 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/ChainingListenableFuture.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/ChainingListenableFuture.java @@ -208,7 +208,7 @@ public void run() { ChainingListenableFuture.this.outputFuture = null; } } - }, MoreExecutors.newDirectExecutorService()); + }, Futures.newExecutorService()); } catch (UndeclaredThrowableException e) { // Set the cause of the exception as this future's exception setException(e.getCause()); diff --git a/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/Futures.java b/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/Futures.java index a882efd5..6d55fbd1 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/Futures.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/Futures.java @@ -26,9 +26,12 @@ import com.google.common.util.concurrent.SettableFuture; import javax.annotation.Nullable; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.util.List; import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.util.concurrent.Uninterruptibles.getUninterruptibly; @@ -74,7 +77,7 @@ public abstract class Futures { * callback in the thread that calls {@code addCallback} or {@code * Future.cancel}. Second, callbacks may run in an internal thread of the * system responsible for the input {@code Future}, such as an RPC network - * thread. Finally, during the execution of a {@code newDirectExecutorService} + * thread. Finally, during the execution of a {@code newExecutorService} * callback, all other registered but unexecuted listeners are prevented from * running, even if those listeners are to run in other executors. * @@ -87,7 +90,7 @@ public abstract class Futures { */ public static void addCallback(ListenableFuture future, FutureCallback callback) { - addCallback(future, callback, MoreExecutors.newDirectExecutorService()); + addCallback(future, callback, newExecutorService()); } /** @@ -115,8 +118,8 @@ public static void addCallback(ListenableFuture future, * * When the callback is fast and lightweight consider {@linkplain * Futures#addCallback(ListenableFuture, FutureCallback) the other overload} - * or explicit use of {@link MoreExecutors#newDirectExecutorService - * newDirectExecutorService}. For heavier callbacks, this choice carries some + * or explicit use of {@link #newExecutorService()}. + * For heavier callbacks, this choice carries some * caveats: First, the thread that the callback runs in depends on whether * the input {@code Future} is done at the time {@code addCallback} is called * and on whether the input {@code Future} is ever cancelled. In particular, @@ -124,7 +127,7 @@ public static void addCallback(ListenableFuture future, * {@code addCallback} or {@code Future.cancel}. Second, callbacks may run in * an internal thread of the system responsible for the input {@code Future}, * such as an RPC network thread. Finally, during the execution of a {@code - * newDirectExecutorService} callback, all other registered but unexecuted + * newExecutorService} callback, all other registered but unexecuted * listeners are prevented from running, even if those listeners are to run * in other executors. * @@ -218,7 +221,7 @@ public static ListenableFuture immediateFailedFuture( * thread that called {@code transform}. Second, transformations may run in * an internal thread of the system responsible for the input {@code Future}, * such as an RPC network thread. Finally, during the execution of a {@code - * newDirectExecutorService} transformation, all other registered but unexecuted + * newExecutorService} transformation, all other registered but unexecuted * listeners are prevented from running, even if those listeners are to run * in other executors. * @@ -240,7 +243,7 @@ public static ListenableFuture immediateFailedFuture( */ public static ListenableFuture transform(ListenableFuture future, final Function function) { - return Futures.transform(future, function, MoreExecutors.newDirectExecutorService()); + return Futures.transform(future, function, newExecutorService()); } /** @@ -272,7 +275,7 @@ public static ListenableFuture transform(ListenableFuture future, *

Note: For cases in which the transformation is fast and lightweight, * consider {@linkplain Futures#transform(ListenableFuture, Function) the * other overload} or explicit use of {@link - * MoreExecutors#newDirectExecutorService}. For heavier transformations, this + * #newExecutorService}. For heavier transformations, this * choice carries some caveats: First, the thread that the transformation * runs in depends on whether the input {@code Future} is done at the time * {@code transform} is called. In particular, if called late, {@code @@ -280,7 +283,7 @@ public static ListenableFuture transform(ListenableFuture future, * {@code transform}. Second, transformations may run in an internal thread * of the system responsible for the input {@code Future}, such as an RPC * network thread. Finally, during the execution of a {@code - * newDirectExecutorService} transformation, all other registered but unexecuted + * newExecutorService} transformation, all other registered but unexecuted * listeners are prevented from running, even if those listeners are to run * in other executors. * @@ -304,6 +307,21 @@ public static ListenableFuture transform(ListenableFuture future, return chain(future, wrapperFunction, executor); } + public static ExecutorService newExecutorService() { + try { + try { + Method method = MoreExecutors.class.getMethod("newDirectExecutorService"); + return (ExecutorService) method.invoke(null); + } catch (NoSuchMethodException e) { + // guava older than 18, fallback to `sameThreadExecutor` + Method method = MoreExecutors.class.getMethod("sameThreadExecutor"); + return (ExecutorService) method.invoke(null); + } + } catch (IllegalAccessException | InvocationTargetException | NoSuchMethodException e ) { + throw new RuntimeException(e); + } + } + /** * Creates a new {@code ListenableFuture} whose value is a list containing the * values of all its input futures, if all succeed. If any input fails, the @@ -324,7 +342,7 @@ public static ListenableFuture transform(ListenableFuture future, public static ListenableFuture> allAsList( ListenableFuture... futures) { return new ListFuture(ImmutableList.copyOf(futures), true, - MoreExecutors.newDirectExecutorService()); + newExecutorService()); } /** @@ -347,7 +365,7 @@ public static ListenableFuture> allAsList( public static ListenableFuture> allAsList( Iterable> futures) { return new ListFuture(ImmutableList.copyOf(futures), true, - MoreExecutors.newDirectExecutorService()); + newExecutorService()); } /** @@ -379,14 +397,14 @@ public static ListenableFuture> allAsList( *

Note: For cases in which the work of creating the derived future is * fast and lightweight, consider {@linkplain Futures#chain(ListenableFuture, * Function) the other overload} or explicit use of {@code - * newDirectExecutorService}. For heavier derivations, this choice carries some + * newExecutorService}. For heavier derivations, this choice carries some * caveats: First, the thread that the derivation runs in depends on whether * the input {@code Future} is done at the time {@code chain} is called. In * particular, if called late, {@code chain} will run the derivation in the * thread that called {@code chain}. Second, derivations may run in an * internal thread of the system responsible for the input {@code Future}, * such as an RPC network thread. Finally, during the execution of a {@code - * newDirectExecutorService} {@code chain} function, all other registered but + * newExecutorService} {@code chain} function, all other registered but * unexecuted listeners are prevented from running, even if those listeners * are to run in other executors. * diff --git a/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/ListFuture.java b/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/ListFuture.java index 44345679..a7c961be 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/ListFuture.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/ListFuture.java @@ -75,7 +75,7 @@ public void run() { // Let go of the memory held by other futures ListFuture.this.futures = null; } - }, MoreExecutors.newDirectExecutorService()); + }, Futures.newExecutorService()); // Now begin the "real" initialization. From f7a4a16363fd36819a86673904c79d3b69e8cfb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Mu=C3=B1iz?= Date: Wed, 3 Mar 2021 17:37:03 +0100 Subject: [PATCH 4/7] Javadocs --- .../plugins/workflow/support/concurrent/Futures.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/Futures.java b/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/Futures.java index 6d55fbd1..cb875840 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/Futures.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/Futures.java @@ -307,6 +307,13 @@ public static ListenableFuture transform(ListenableFuture future, return chain(future, wrapperFunction, executor); } + /** + * Returns an {@link ExecutorService} to be used as a parameter in other methods. + * It calls {@code MoreExecutors#newDirectExecutorService} or falls back to {@code MoreExecutors#sameThreadExecutor} + * for compatibility with older (< 18.0) versions of guava. + * + * @since TODO + */ public static ExecutorService newExecutorService() { try { try { From 2ff560ad59350be8d087ec5287d1e2e3bec33e93 Mon Sep 17 00:00:00 2001 From: Tim Jacomb <21194782+timja@users.noreply.github.com> Date: Wed, 3 Mar 2021 16:45:15 +0000 Subject: [PATCH 5/7] Remove unneeded exclusion --- pom.xml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pom.xml b/pom.xml index 191367b3..5a9439d9 100644 --- a/pom.xml +++ b/pom.xml @@ -98,12 +98,6 @@ org.jenkins-ci.plugins script-security - - - org.checkerframework - checker-qual - - From f274d5dd5b9709ca75e893024bbef51c7478c2a3 Mon Sep 17 00:00:00 2001 From: Tim Jacomb <21194782+timja@users.noreply.github.com> Date: Wed, 3 Mar 2021 17:12:27 +0000 Subject: [PATCH 6/7] Apply suggestions from code review Co-authored-by: Jesse Glick --- .../plugins/workflow/support/concurrent/Futures.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/Futures.java b/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/Futures.java index cb875840..9ddbb4ac 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/Futures.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/Futures.java @@ -310,7 +310,7 @@ public static ListenableFuture transform(ListenableFuture future, /** * Returns an {@link ExecutorService} to be used as a parameter in other methods. * It calls {@code MoreExecutors#newDirectExecutorService} or falls back to {@code MoreExecutors#sameThreadExecutor} - * for compatibility with older (< 18.0) versions of guava. + * for compatibility with older (< 18.0) versions of guava. * * @since TODO */ @@ -320,7 +320,7 @@ public static ExecutorService newExecutorService() { Method method = MoreExecutors.class.getMethod("newDirectExecutorService"); return (ExecutorService) method.invoke(null); } catch (NoSuchMethodException e) { - // guava older than 18, fallback to `sameThreadExecutor` + // Guava older than 18, fall back to `sameThreadExecutor` Method method = MoreExecutors.class.getMethod("sameThreadExecutor"); return (ExecutorService) method.invoke(null); } From c5ecc11733196e2dc054a899e846be31e8a0b1e4 Mon Sep 17 00:00:00 2001 From: Tim Jacomb Date: Wed, 3 Mar 2021 18:52:36 +0000 Subject: [PATCH 7/7] Invert guava method --- .../plugins/workflow/support/concurrent/Futures.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/Futures.java b/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/Futures.java index 9ddbb4ac..9b313f24 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/Futures.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/support/concurrent/Futures.java @@ -317,11 +317,12 @@ public static ListenableFuture transform(ListenableFuture future, public static ExecutorService newExecutorService() { try { try { - Method method = MoreExecutors.class.getMethod("newDirectExecutorService"); + // Guava older than 18 + Method method = MoreExecutors.class.getMethod("sameThreadExecutor"); return (ExecutorService) method.invoke(null); } catch (NoSuchMethodException e) { - // Guava older than 18, fall back to `sameThreadExecutor` - Method method = MoreExecutors.class.getMethod("sameThreadExecutor"); + // TODO invert this to prefer the newer guava method once guava is upgrade in Jenkins core + Method method = MoreExecutors.class.getMethod("newDirectExecutorService"); return (ExecutorService) method.invoke(null); } } catch (IllegalAccessException | InvocationTargetException | NoSuchMethodException e ) {