This is an automated email from the ASF dual-hosted git repository.
Aias00 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shenyu.git
The following commit(s) were added to refs/heads/master by this push:
new fff85f433b fix(logging): shutdown hook cleanup reaches close0 when
hook removal throws (#7686) (#7699)
fff85f433b is described below
commit fff85f433bf21c081e366dd34826c3e87e29debd
Author: konlue <[email protected]>
AuthorDate: Mon Oct 19 09:38:59 2026 +0800
fix(logging): shutdown hook cleanup reaches close0 when hook removal throws
(#7686) (#7699)
---
.../common/client/AbstractLogConsumeClient.java | 36 +++++++++--
.../client/AbstractLogConsumeClientTest.java | 71 ++++++++++++++++++++++
2 files changed, 102 insertions(+), 5 deletions(-)
diff --git
a/shenyu-plugin/shenyu-plugin-logging/shenyu-plugin-logging-common/src/main/java/org/apache/shenyu/plugin/logging/common/client/AbstractLogConsumeClient.java
b/shenyu-plugin/shenyu-plugin-logging/shenyu-plugin-logging-common/src/main/java/org/apache/shenyu/plugin/logging/common/client/AbstractLogConsumeClient.java
index 86e6dda16a..81ba6c2067 100644
---
a/shenyu-plugin/shenyu-plugin-logging/shenyu-plugin-logging-common/src/main/java/org/apache/shenyu/plugin/logging/common/client/AbstractLogConsumeClient.java
+++
b/shenyu-plugin/shenyu-plugin-logging/shenyu-plugin-logging-common/src/main/java/org/apache/shenyu/plugin/logging/common/client/AbstractLogConsumeClient.java
@@ -83,11 +83,8 @@ public abstract class AbstractLogConsumeClient<T extends
GenericGlobalConfig, L
@Override
public void close() {
- if (!ObjectUtils.isEmpty(closeThread.get())) {
- Runtime.getRuntime().removeShutdownHook(closeThread.get());
- }
- if (isStarted.get()) {
- isStarted.set(false);
+ this.tryRemoveShutdownHook();
+ if (isStarted.compareAndSet(true, false)) {
try {
this.close0();
} catch (Exception e) {
@@ -96,6 +93,35 @@ public abstract class AbstractLogConsumeClient<T extends
GenericGlobalConfig, L
}
}
+ /**
+ * Remove the registered shutdown hook. When the JVM is already shutting
down,
+ * the hook thread is running this {@code close()} itself, so removal
throws
+ * {@link IllegalStateException} and is neither possible nor needed;
cleanup
+ * must still proceed to {@code close0()}.
+ */
+ protected void tryRemoveShutdownHook() {
+ final Thread hook = closeThread.get();
+ if (ObjectUtils.isEmpty(hook)) {
+ return;
+ }
+ try {
+ removeHook(hook);
+ } catch (IllegalStateException e) {
+ LOG.debug("{} skips shutdown-hook removal: {}",
this.getClass().getSimpleName(), e.getMessage());
+ }
+ }
+
+ /**
+ * Single point that talks to {@link Runtime}; protected so tests can
simulate
+ * the {@code IllegalStateException("Shutdown in progress")} thrown when
the JVM
+ * is already shutting down and the hook thread is running {@code close()}
itself.
+ *
+ * @param hook the shutdown hook registered at init
+ */
+ protected void removeHook(final Thread hook) {
+ Runtime.getRuntime().removeShutdownHook(hook);
+ }
+
@Override
public void consume(final List<L> logs) throws Exception {
if (CollectionUtils.isEmpty(logs) || !isStarted.get()) {
diff --git
a/shenyu-plugin/shenyu-plugin-logging/shenyu-plugin-logging-common/src/test/java/org/apache/shenyu/plugin/logging/common/client/AbstractLogConsumeClientTest.java
b/shenyu-plugin/shenyu-plugin-logging/shenyu-plugin-logging-common/src/test/java/org/apache/shenyu/plugin/logging/common/client/AbstractLogConsumeClientTest.java
index 67089e3844..1d4143d47e 100644
---
a/shenyu-plugin/shenyu-plugin-logging/shenyu-plugin-logging-common/src/test/java/org/apache/shenyu/plugin/logging/common/client/AbstractLogConsumeClientTest.java
+++
b/shenyu-plugin/shenyu-plugin-logging/shenyu-plugin-logging-common/src/test/java/org/apache/shenyu/plugin/logging/common/client/AbstractLogConsumeClientTest.java
@@ -23,6 +23,7 @@ import org.junit.jupiter.api.Test;
import java.util.Collections;
import java.util.List;
+import java.util.concurrent.atomic.AtomicReference;
import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -57,6 +58,36 @@ public final class AbstractLogConsumeClientTest {
assertEquals(1, client.getCloseCount());
}
+ @Test
+ public void testClose0StillRunsWhenHookRemovalFails() {
+ ThrowingHookLogConsumeClient client = new
ThrowingHookLogConsumeClient();
+
+ try {
+ client.initClient(new GenericGlobalConfig());
+ client.close();
+ } finally {
+
Runtime.getRuntime().removeShutdownHook(client.getRegisteredHook());
+ }
+
+ assertEquals(1, client.getCloseCount());
+ }
+
+ @Test
+ public void testCloseIsIdempotent() {
+ TestLogConsumeClient client = new TestLogConsumeClient(true);
+
+ try {
+ client.initClient(new GenericGlobalConfig());
+ client.close();
+ client.close();
+ } finally {
+ // hook already removed by the first close(); removal is a no-op
now
+ client.close();
+ }
+
+ assertEquals(1, client.getCloseCount());
+ }
+
@Test
public void testFailedReinitializationLeavesClientStopped() throws
Exception {
TestLogConsumeClient client = new TestLogConsumeClient(true);
@@ -109,4 +140,44 @@ public final class AbstractLogConsumeClientTest {
return closeCount;
}
}
+
+ /**
+ * Simulates the JVM-shutdown scenario where {@code removeShutdownHook}
throws
+ * {@link IllegalStateException}: cleanup must still reach {@code
close0()}.
+ */
+ private static final class ThrowingHookLogConsumeClient extends
AbstractLogConsumeClient<GenericGlobalConfig, ShenyuRequestLog> {
+
+ private final AtomicReference<Thread> registeredHook = new
AtomicReference<>();
+
+ private int closeCount;
+
+ @Override
+ public boolean initClient0(final GenericGlobalConfig config) {
+ registeredHook.set(new Thread(this::close));
+ return true;
+ }
+
+ @Override
+ public void consume0(final List<ShenyuRequestLog> logs) {
+ // not used in this test
+ }
+
+ @Override
+ public void close0() {
+ closeCount++;
+ }
+
+ @Override
+ protected void removeHook(final Thread hook) {
+ throw new IllegalStateException("Shutdown in progress");
+ }
+
+ private Thread getRegisteredHook() {
+ return registeredHook.get();
+ }
+
+ private int getCloseCount() {
+ return closeCount;
+ }
+ }
}
