This is an automated email from the ASF dual-hosted git repository. ggregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-exec.git
commit b78d1063ab3796edba23ca17544eeebc766334bb Author: Gary D. Gregory <[email protected]> AuthorDate: Sun Aug 17 10:54:48 2025 -0400 ExecuteWatchdog.builder().get() now uses a default timeout of 30 seconds instead of throwing a NullPointerException - Calling org.apache.commons.exec.ExecuteWatchdog.Builder.setTimeout(Duration) with null now resets to the default INFINITE_TIMEOUT_DURATION timeout - Add org.apache.commons.exec.Watchdog.getTimeout() - Calling org.apache.commons.exec.Watchdog.Builder.setThreadFactory(ThreadFactory) with null now resets to the default java.util.concurrent.Executors.defaultThreadFactory() - Calling org.apache.commons.exec.ExecuteWatchdog.Builder.setThreadFactory(ThreadFactory) with null now resets to the default java.util.concurrent.Executors.defaultThreadFactory() --- src/changes/changes.xml | 7 ++- .../org/apache/commons/exec/ExecuteWatchdog.java | 45 ++++++++++------- .../java/org/apache/commons/exec/Watchdog.java | 56 +++++++++++++++------- ...{WatchdogTest.java => ExecuteWatchdogTest.java} | 17 +++++-- .../java/org/apache/commons/exec/WatchdogTest.java | 9 +++- 5 files changed, 94 insertions(+), 40 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index e97f7bbd..dbd99a83 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -26,10 +26,15 @@ <release version="1.6.0" date="YYYY-MM-DD" description="This is a feature and maintenance release. Java 8 or later is required."> <!-- FIX --> <action type="fix" dev="ggregory" due-to="Gary Gregory">Watchdog.builder().get() now uses a default timeout of 30 seconds instead of throwing a NullPointerException.</action> - <action type="fix" dev="ggregory" due-to="Gary Gregory">Calling org.apache.commons.exec.Watchdog.Builder.setTimeout(Duration) with null now resets to the default 30 seconds timeout.</action> + <action type="fix" dev="ggregory" due-to="Gary Gregory">ExecuteWatchdog.builder().get() now uses a default timeout of 30 seconds instead of throwing a NullPointerException.</action> + <action type="fix" dev="ggregory" due-to="Gary Gregory">Calling org.apache.commons.exec.Watchdog.Builder.setTimeout(Duration) with null now resets to the default INFINITE_TIMEOUT_DURATION timeout.</action> + <action type="fix" dev="ggregory" due-to="Gary Gregory">Calling org.apache.commons.exec.ExecuteWatchdog.Builder.setTimeout(Duration) with null now resets to the default INFINITE_TIMEOUT_DURATION timeout.</action> + <action type="fix" dev="ggregory" due-to="Gary Gregory">Calling org.apache.commons.exec.Watchdog.Builder.setThreadFactory(ThreadFactory) with null now resets to the default java.util.concurrent.Executors.defaultThreadFactory().</action> + <action type="fix" dev="ggregory" due-to="Gary Gregory">Calling org.apache.commons.exec.ExecuteWatchdog.Builder.setThreadFactory(ThreadFactory) with null now resets to the default java.util.concurrent.Executors.defaultThreadFactory().</action> <action type="fix" dev="ggregory" due-to="Gary Gregory">Fix Checkstyle issues.</action> <!-- ADD --> <action type="add" dev="ggregory" due-to="Gary Gregory">TimeoutObserver now extends Consumer<Watchdog>.</action> + <action type="add" dev="ggregory" due-to="Gary Gregory">Add org.apache.commons.exec.Watchdog.getTimeout().</action> <!-- UPDATE --> <action type="update" dev="ggregory" due-to="Dependabot, Gary Gregory">Bump org.apache.commons:commons-parent from 83 to 85.</action> <action type="update" dev="ggregory" due-to="Dependabot, Gary Gregory">Bump org.apache.commons:commons-lang3 from 3.17.0 to 3.18.0 #282.</action> diff --git a/src/main/java/org/apache/commons/exec/ExecuteWatchdog.java b/src/main/java/org/apache/commons/exec/ExecuteWatchdog.java index 7853b440..9bc89014 100644 --- a/src/main/java/org/apache/commons/exec/ExecuteWatchdog.java +++ b/src/main/java/org/apache/commons/exec/ExecuteWatchdog.java @@ -61,10 +61,10 @@ public class ExecuteWatchdog implements TimeoutObserver { public static final class Builder implements Supplier<ExecuteWatchdog> { /** Thread factory. */ - private ThreadFactory threadFactory; + private ThreadFactory threadFactory = Executors.defaultThreadFactory(); /** Timeout duration. */ - private Duration timeout; + private Duration timeout = INFINITE_TIMEOUT_DURATION; /** * Constructs a new instance. @@ -86,22 +86,22 @@ public class ExecuteWatchdog implements TimeoutObserver { /** * Sets the thread factory. * - * @param threadFactory the thread factory. + * @param threadFactory the thread factory, null resets to the default {@link Executors#defaultThreadFactory()}. * @return {@code this} instance. */ public Builder setThreadFactory(final ThreadFactory threadFactory) { - this.threadFactory = threadFactory; + this.threadFactory = threadFactory != null ? threadFactory : Executors.defaultThreadFactory(); return this; } /** * Sets the timeout duration. * - * @param timeout the timeout duration. + * @param timeout the timeout duration, null resets to default {@link #INFINITE_TIMEOUT_DURATION}. * @return {@code this} instance. */ public Builder setTimeout(final Duration timeout) { - this.timeout = timeout; + this.timeout = timeout != null ? timeout : INFINITE_TIMEOUT_DURATION; return this; } @@ -149,17 +149,6 @@ public class ExecuteWatchdog implements TimeoutObserver { /** Will tell us whether timeout has occurred. */ private final Watchdog watchdog; - /** - * Creates a new watchdog with a given timeout. - * - * @param timeoutMillis the timeout for the process in milliseconds. It must be greater than 0 or {@code INFINITE_TIMEOUT}. - * @deprecated Use {@link Builder#get()}. - */ - @Deprecated - public ExecuteWatchdog(final long timeoutMillis) { - this(builder().setTimeout(Duration.ofMillis(timeoutMillis))); - } - /** * Creates a new watchdog with a given timeout. * @@ -171,7 +160,7 @@ public class ExecuteWatchdog implements TimeoutObserver { this.watch = false; this.hasWatchdog = !INFINITE_TIMEOUT_DURATION.equals(builder.timeout); this.processStarted = false; - this.threadFactory = builder.threadFactory != null ? builder.threadFactory : Executors.defaultThreadFactory(); + this.threadFactory = builder.threadFactory; if (this.hasWatchdog) { this.watchdog = Watchdog.builder().setThreadFactory(threadFactory).setTimeout(builder.timeout).get(); this.watchdog.addTimeoutObserver(this); @@ -180,6 +169,17 @@ public class ExecuteWatchdog implements TimeoutObserver { } } + /** + * Creates a new watchdog with a given timeout. + * + * @param timeoutMillis the timeout for the process in milliseconds. It must be greater than 0 or {@code INFINITE_TIMEOUT}. + * @deprecated Use {@link Builder#get()}. + */ + @Deprecated + public ExecuteWatchdog(final long timeoutMillis) { + this(builder().setTimeout(Duration.ofMillis(timeoutMillis))); + } + /** * This method will rethrow the exception that was possibly caught during the run of the process. It will only remain valid once the process has been * terminated either by 'error', timeout or manual intervention. Information will be discarded once a new process is run. @@ -234,6 +234,15 @@ public class ExecuteWatchdog implements TimeoutObserver { notifyAll(); } + /** + * Gets the watchdog. + * + * @return the watchdog. + */ + Watchdog getWatchdog() { + return watchdog; + } + /** * Tests whether the watchdog is still monitoring the process. * diff --git a/src/main/java/org/apache/commons/exec/Watchdog.java b/src/main/java/org/apache/commons/exec/Watchdog.java index 84c80bde..ee07af32 100644 --- a/src/main/java/org/apache/commons/exec/Watchdog.java +++ b/src/main/java/org/apache/commons/exec/Watchdog.java @@ -22,6 +22,7 @@ package org.apache.commons.exec; import java.time.Duration; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.Executors; import java.util.concurrent.ThreadFactory; import java.util.function.Supplier; @@ -39,10 +40,13 @@ public class Watchdog implements Runnable { */ public static final class Builder implements Supplier<Watchdog> { + /** + * Default timeout. + */ private static final Duration DEFAULT_TIMEOUT = Duration.ofSeconds(30); /** Thread factory. */ - private ThreadFactory threadFactory; + private ThreadFactory threadFactory = Executors.defaultThreadFactory(); /** * Timeout duration. @@ -69,11 +73,11 @@ public class Watchdog implements Runnable { /** * Sets the thread factory. * - * @param threadFactory the thread factory. + * @param threadFactory the thread factory, null resets to the default {@link Executors#defaultThreadFactory()}. * @return {@code this} instance. */ public Builder setThreadFactory(final ThreadFactory threadFactory) { - this.threadFactory = threadFactory; + this.threadFactory = threadFactory != null ? threadFactory : Executors.defaultThreadFactory(); return this; } @@ -108,7 +112,7 @@ public class Watchdog implements Runnable { /** * Timeout duration. */ - private final long timeoutMillis; + private final Duration timeout; /** * Whether this is stopped. @@ -120,17 +124,6 @@ public class Watchdog implements Runnable { */ private final ThreadFactory threadFactory; - /** - * Constructs a new instance. - * - * @param timeoutMillis the timeout duration. - * @deprecated Use {@link Builder#get()}. - */ - @Deprecated - public Watchdog(final long timeoutMillis) { - this(builder().setTimeout(Duration.ofMillis(timeoutMillis))); - } - /** * Constructs a new instance. * @@ -141,10 +134,21 @@ public class Watchdog implements Runnable { if (builder.timeout.isNegative() || Duration.ZERO.equals(builder.timeout)) { throw new IllegalArgumentException("Timeout must be positive."); } - this.timeoutMillis = builder.timeout.toMillis(); + this.timeout = builder.timeout; this.threadFactory = builder.threadFactory; } + /** + * Constructs a new instance. + * + * @param timeoutMillis the timeout duration. + * @deprecated Use {@link Builder#get()}. + */ + @Deprecated + public Watchdog(final long timeoutMillis) { + this(builder().setTimeout(Duration.ofMillis(timeoutMillis))); + } + /** * Adds a TimeoutObserver. * @@ -161,6 +165,25 @@ public class Watchdog implements Runnable { observers.forEach(o -> o.timeoutOccured(this)); } + /** + * Gets the thread factory. + * + * @return the thread factory. + */ + ThreadFactory getThreadFactory() { + return threadFactory; + } + + /** + * Gets the timeout. + * + * @return the timeout. + * @since 1.6.0 + */ + public Duration getTimeout() { + return timeout; + } + /** * Removes a TimeoutObserver. * @@ -175,6 +198,7 @@ public class Watchdog implements Runnable { final long startTimeMillis = System.currentTimeMillis(); boolean isWaiting; synchronized (this) { + final long timeoutMillis = timeout.toMillis(); long timeLeftMillis = timeoutMillis - (System.currentTimeMillis() - startTimeMillis); isWaiting = timeLeftMillis > 0; while (!stopped && isWaiting) { diff --git a/src/test/java/org/apache/commons/exec/WatchdogTest.java b/src/test/java/org/apache/commons/exec/ExecuteWatchdogTest.java similarity index 56% copy from src/test/java/org/apache/commons/exec/WatchdogTest.java copy to src/test/java/org/apache/commons/exec/ExecuteWatchdogTest.java index 9d155e16..396ca025 100644 --- a/src/test/java/org/apache/commons/exec/WatchdogTest.java +++ b/src/test/java/org/apache/commons/exec/ExecuteWatchdogTest.java @@ -19,17 +19,26 @@ package org.apache.commons.exec; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + +import java.time.Duration; import org.junit.jupiter.api.Test; /** - * Tests {@link Watchdog}. + * Tests {@link ExecuteWatchdog}. */ -class WatchdogTest { - +class ExecuteWatchdogTest { + @Test void testBuilder() { - assertNotNull(Watchdog.builder().get()); + assertNotNull(ExecuteWatchdog.builder().get()); + assertNull(ExecuteWatchdog.builder().get().getWatchdog()); + assertNotNull(ExecuteWatchdog.builder().setTimeout(null).get()); + assertEquals(Duration.ofMinutes(1), ExecuteWatchdog.builder().setTimeout(Duration.ofMinutes(1)).get().getWatchdog().getTimeout()); + assertNotNull(ExecuteWatchdog.builder().setThreadFactory(null).get()); + assertNotNull(ExecuteWatchdog.builder().setThreadFactory(null).setTimeout(Duration.ofMinutes(1)).get().getWatchdog().getThreadFactory()); } } diff --git a/src/test/java/org/apache/commons/exec/WatchdogTest.java b/src/test/java/org/apache/commons/exec/WatchdogTest.java index 9d155e16..d0cd2a20 100644 --- a/src/test/java/org/apache/commons/exec/WatchdogTest.java +++ b/src/test/java/org/apache/commons/exec/WatchdogTest.java @@ -19,17 +19,24 @@ package org.apache.commons.exec; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import java.time.Duration; + import org.junit.jupiter.api.Test; /** * Tests {@link Watchdog}. */ class WatchdogTest { - + @Test void testBuilder() { assertNotNull(Watchdog.builder().get()); + assertNotNull(Watchdog.builder().setTimeout(null).get()); + assertEquals(Duration.ofMinutes(1), Watchdog.builder().setTimeout(Duration.ofMinutes(1)).get().getTimeout()); + assertNotNull(Watchdog.builder().setThreadFactory(null).get()); + assertNotNull(Watchdog.builder().setThreadFactory(null).get().getThreadFactory()); } }
