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 80eca0445987a0297258ad4f8c143fc4017ab2d3
Author: Gary D. Gregory <garydgreg...@gmail.com>
AuthorDate: Wed Apr 30 10:01:26 2025 -0400
Fix SpotBugs AT_STALE_THREAD_WRITE_OF_PRIMITIVE: Shared primitive
variable "added" in one thread may not yield the value of the most
recent write from another thread
[org.apache.commons.exec.ShutdownHookProcessDestroyer]
---
src/changes/changes.xml | 3 ++-
.../commons/exec/ShutdownHookProcessDestroyer.java | 21 ++++++++++-----------
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index a70843a3..a2d7da1c 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -42,7 +42,8 @@
<action type="fix" dev="ggregory" due-to="Gary Gregory">Deprecate
MapUtils.MapUtils().</action>
<action type="fix" dev="ggregory" due-to="Gary Gregory">Deprecate
StringUtils.StringUtils().</action>
<action type="fix" dev="ggregory" due-to="Gary Gregory">Fix
Javadoc warnings.</action>
- <action type="fix" dev="ggregory" due-to="Gary Gregory">Fix
SpotBugs AT_STALE_THREAD_WRITE_OF_PRIMITIVE: Shared primitive variable
"shouldDestroy" in one thread may not yield the value of the most recent write
from another thread
[org.apache.commons.exec.ShutdownHookProcessDestroyer$ProcessDestroyerThread].</action>
+ <action type="fix" dev="ggregory" due-to="Gary Gregory,
SpotBugs">Fix SpotBugs AT_STALE_THREAD_WRITE_OF_PRIMITIVE: Shared primitive
variable "shouldDestroy" in one thread may not yield the value of the most
recent write from another thread
[org.apache.commons.exec.ShutdownHookProcessDestroyer$ProcessDestroyerThread].</action>
+ <action type="fix" dev="ggregory" due-to="Gary Gregory,
SpotBugs">Fix SpotBugs AT_STALE_THREAD_WRITE_OF_PRIMITIVE: Shared primitive
variable "added" in one thread may not yield the value of the most recent write
from another thread
[org.apache.commons.exec.ShutdownHookProcessDestroyer].</action>
<!-- UPDATE -->
<action type="update" dev="ggregory" due-to="Dependabot, Gary
Gregory">Bump org.apache.commons:commons-parent from 65 to 81 #174, #204, #212,
#214, #219, #223, #226, #233, #253.</action>
</release>
diff --git
a/src/main/java/org/apache/commons/exec/ShutdownHookProcessDestroyer.java
b/src/main/java/org/apache/commons/exec/ShutdownHookProcessDestroyer.java
index 49e337df..b8879389 100644
--- a/src/main/java/org/apache/commons/exec/ShutdownHookProcessDestroyer.java
+++ b/src/main/java/org/apache/commons/exec/ShutdownHookProcessDestroyer.java
@@ -41,7 +41,7 @@ public class ShutdownHookProcessDestroyer implements
ProcessDestroyer, Runnable
}
public void setShouldDestroy(final boolean shouldDestroy) {
- this.shouldDestroy.set(shouldDestroy);
+ this.shouldDestroy.compareAndSet(!shouldDestroy, shouldDestroy);
}
}
@@ -52,12 +52,12 @@ public class ShutdownHookProcessDestroyer implements
ProcessDestroyer, Runnable
private ProcessDestroyerThread destroyProcessThread;
/** Whether or not this ProcessDestroyer has been registered as a shutdown
hook. */
- private boolean added;
+ private AtomicBoolean added = new AtomicBoolean();
/**
* Whether or not this ProcessDestroyer is currently running as shutdown
hook.
*/
- private volatile boolean running;
+ private AtomicBoolean running = new AtomicBoolean();
/**
* Constructs a {@code ProcessDestroyer} and obtains {@code
Runtime.addShutdownHook()} and {@code Runtime.removeShutdownHook()} through
reflection. The
@@ -90,10 +90,10 @@ public class ShutdownHookProcessDestroyer implements
ProcessDestroyer, Runnable
* Registers this {@code ProcessDestroyer} as a shutdown hook.
*/
private void addShutdownHook() {
- if (!running) {
+ if (!running.get()) {
destroyProcessThread = new ProcessDestroyerThread();
Runtime.getRuntime().addShutdownHook(destroyProcessThread);
- added = true;
+ added.compareAndSet(false, true);
}
}
@@ -103,7 +103,7 @@ public class ShutdownHookProcessDestroyer implements
ProcessDestroyer, Runnable
* @return true if this is currently added as shutdown hook.
*/
public boolean isAddedAsShutdownHook() {
- return added;
+ return added.get();
}
/**
@@ -134,17 +134,16 @@ public class ShutdownHookProcessDestroyer implements
ProcessDestroyer, Runnable
}
/**
- * Removes this {@code ProcessDestroyer} as a shutdown hook, uses
reflection to ensure pre-JDK 1.3 compatibility
+ * Removes this {@code ProcessDestroyer} as a shutdown hook.
*/
private void removeShutdownHook() {
- if (added && !running) {
+ if (added.get() && !running.get()) {
final boolean removed =
Runtime.getRuntime().removeShutdownHook(destroyProcessThread);
if (!removed) {
System.err.println("Could not remove shutdown hook");
}
// start the hook thread, a unstarted thread may not be eligible
for garbage collection Cf.: https://developer.java.sun.com/developer/
// bugParade/bugs/4533087.html
-
destroyProcessThread.setShouldDestroy(false);
destroyProcessThread.start();
// this should return quickly, since it basically is a NO-OP.
@@ -155,7 +154,7 @@ public class ShutdownHookProcessDestroyer implements
ProcessDestroyer, Runnable
// it should not kill any processes unexpectedly
}
destroyProcessThread = null;
- added = false;
+ added.compareAndSet(true, false);
}
}
@@ -165,7 +164,7 @@ public class ShutdownHookProcessDestroyer implements
ProcessDestroyer, Runnable
@Override
public void run() {
synchronized (processes) {
- running = true;
+ running.compareAndSet(false, true);
processes.forEach(process -> {
try {
process.destroy();
