This is an automated email from the ASF dual-hosted git repository.

zihaoxiang pushed a commit to branch dev
in repository https://gitbox.apache.org/repos/asf/dolphinscheduler.git


The following commit(s) were added to refs/heads/dev by this push:
     new 41d4e4c523 [Fix-17753][Task-Plugin] Fix workflow killed not success 
when shell is sh (#17776)
41d4e4c523 is described below

commit 41d4e4c52391e315026f8db1a142d772711a3fc9
Author: Qiong Zhou <[email protected]>
AuthorDate: Wed Dec 10 10:32:13 2025 +0800

    [Fix-17753][Task-Plugin] Fix workflow killed not success when shell is sh 
(#17776)
---
 .../plugin/task/api/utils/ProcessUtils.java        | 27 ++++---
 .../plugin/task/api/utils/ProcessUtilsTest.java    | 84 ++++++++++++++--------
 2 files changed, 71 insertions(+), 40 deletions(-)

diff --git 
a/dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/utils/ProcessUtils.java
 
b/dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/utils/ProcessUtils.java
index 2937a4a0d2..c304db0e76 100644
--- 
a/dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/utils/ProcessUtils.java
+++ 
b/dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/utils/ProcessUtils.java
@@ -99,6 +99,10 @@ public final class ProcessUtils {
      */
     private static final Pattern PID_PATTERN = Pattern.compile("\\s+");
 
+    private static final String SIGINT = "2";
+    private static final String SIGTERM = "15";
+    private static final String SIGKILL = "9";
+
     /**
      * Terminate the task process, support multi-level signal processing and 
fallback strategy
      * @param request Task execution context
@@ -116,27 +120,28 @@ public final class ProcessUtils {
             // Get all child processes
             List<Integer> pidList = getPidList(processId);
 
-            // 1. Try to terminate gracefully (SIGINT)
-            boolean gracefulKillSuccess = sendKillSignal("SIGINT", pidList, 
request.getTenantCode());
+            // 1. Try to terminate gracefully `kill -2`
+            boolean gracefulKillSuccess = sendKillSignal(SIGINT, pidList, 
request.getTenantCode());
             if (gracefulKillSuccess) {
-                log.info("Successfully killed process tree using SIGINT, 
processId: {}", processId);
+                log.info("Successfully killed process tree by SIGINT, 
processId: {}", processId);
                 return true;
             }
 
-            // 2. Try to terminate forcefully (SIGTERM)
-            boolean termKillSuccess = sendKillSignal("SIGTERM", pidList, 
request.getTenantCode());
+            // 2. Try to terminate gracefully `kill -15`
+            boolean termKillSuccess = sendKillSignal(SIGTERM, pidList, 
request.getTenantCode());
             if (termKillSuccess) {
-                log.info("Successfully killed process tree using SIGTERM, 
processId: {}", processId);
+                log.info("Successfully killed process tree by SIGTERM, 
processId: {}", processId);
                 return true;
             }
 
             // 3. As a last resort, use `kill -9`
-            log.warn("SIGINT & SIGTERM failed, using SIGKILL as a last resort 
for processId: {}", processId);
-            boolean forceKillSuccess = sendKillSignal("SIGKILL", pidList, 
request.getTenantCode());
+            log.warn("Killing process by SIGINT & SIGTERM failed, using 
SIGKILL as a last resort for processId: {}",
+                    processId);
+            boolean forceKillSuccess = sendKillSignal(SIGKILL, pidList, 
request.getTenantCode());
             if (forceKillSuccess) {
-                log.info("Successfully sent SIGKILL signal to process tree, 
processId: {}", processId);
+                log.info("Successfully killed process tree by SIGKILL, 
processId: {}", processId);
             } else {
-                log.error("Error sending SIGKILL signal to process tree, 
processId: {}", processId);
+                log.error("Error killing process tree by SIGKILL, processId: 
{}", processId);
             }
             return forceKillSuccess;
 
@@ -170,7 +175,7 @@ public final class ProcessUtils {
 
         try {
             // 1. Send the kill signal
-            String killCmd = String.format("kill -s %s %s", signal, pids);
+            String killCmd = String.format("kill -%s %s", signal, pids);
             killCmd = OSUtils.getSudoCmd(tenantCode, killCmd);
             log.info("Sending {} to process group: {}, command: {}", signal, 
pids, killCmd);
             OSUtils.exeCmd(killCmd);
diff --git 
a/dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/test/java/org/apache/dolphinscheduler/plugin/task/api/utils/ProcessUtilsTest.java
 
b/dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/test/java/org/apache/dolphinscheduler/plugin/task/api/utils/ProcessUtilsTest.java
index 0af5738ccb..50f5847b00 100644
--- 
a/dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/test/java/org/apache/dolphinscheduler/plugin/task/api/utils/ProcessUtilsTest.java
+++ 
b/dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/test/java/org/apache/dolphinscheduler/plugin/task/api/utils/ProcessUtilsTest.java
@@ -122,8 +122,16 @@ public class ProcessUtilsTest {
         Mockito.when(taskRequest.getTenantCode()).thenReturn("testTenant");
 
         // Mock getPidsStr
-        mockedOSUtils.when(() -> 
OSUtils.exeCmd(Mockito.matches(".*pstree.*12345")))
-                .thenReturn("sudo(12345)---86.sh(1234)");
+        String pstreeCmd;
+        String pstreeOutput;
+        if (SystemUtils.IS_OS_MAC) {
+            pstreeCmd = "pstree -sp 12345";
+            pstreeOutput = "-+= 12345 sudo -+- 1234 86.sh";
+        } else {
+            pstreeCmd = "pstree -p 12345";
+            pstreeOutput = "sudo(12345)---86.sh(1234)";
+        }
+        mockedOSUtils.when(() -> 
OSUtils.exeCmd(pstreeCmd)).thenReturn(pstreeOutput);
 
         // Mock kill -0
         mockedOSUtils.when(() -> OSUtils.getSudoCmd(Mockito.eq("testTenant"), 
Mockito.matches("kill -0.*")))
@@ -138,9 +146,9 @@ public class ProcessUtilsTest {
         Assertions.assertTrue(result);
 
         // Verify SIGINT, SIGTERM, SIGKILL never called
-        mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -s SIGINT 12345"), 
Mockito.never());
-        mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -s SIGTERM 12345"), 
Mockito.never());
-        mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -s SIGKILL 12345"), 
Mockito.never());
+        mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -2 12345 1234"), 
Mockito.never());
+        mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -15 12345 1234"), 
Mockito.never());
+        mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -9 12345 1234"), 
Mockito.never());
     }
 
     @Test
@@ -151,17 +159,26 @@ public class ProcessUtilsTest {
         Mockito.when(taskRequest.getTenantCode()).thenReturn("testTenant");
 
         // Mock getPidsStr
-        mockedOSUtils.when(() -> 
OSUtils.exeCmd(Mockito.matches(".*pstree.*12345")))
-                .thenReturn("sudo(12345)---86.sh(1234)");
+        String pstreeCmd;
+        String pstreeOutput;
+        if (SystemUtils.IS_OS_MAC) {
+            pstreeCmd = "pstree -sp 12345";
+            pstreeOutput = "-+= 12345 sudo -+- 1234 86.sh";
+        } else {
+            pstreeCmd = "pstree -p 12345";
+            pstreeOutput = "sudo(12345)---86.sh(1234)";
+        }
+        mockedOSUtils.when(() -> 
OSUtils.exeCmd(pstreeCmd)).thenReturn(pstreeOutput);
 
         // Mock SIGINT command
-        mockedOSUtils.when(() -> OSUtils.getSudoCmd(Mockito.eq("testTenant"), 
Mockito.matches("kill -s SIGINT.*")))
-                .thenReturn("kill -s SIGINT 12345");
-        mockedOSUtils.when(() -> OSUtils.exeCmd("kill -s SIGINT 
12345")).thenReturn("");
+        mockedOSUtils.when(() -> OSUtils.getSudoCmd(Mockito.eq("testTenant"), 
Mockito.matches("kill -2.*")))
+                .thenReturn("kill -2 12345 1234");
+        mockedOSUtils.when(() -> OSUtils.exeCmd("kill -2 12345 
1234")).thenReturn("");
 
         // Mock kill -0
         mockedOSUtils.when(() -> OSUtils.getSudoCmd(Mockito.eq("testTenant"), 
Mockito.matches("kill -0.*")))
-                .thenReturn("kill -0 12345");
+                .thenReturn("kill -0 12345")
+                .thenReturn("kill -0 1234");
         // Mock the static method OSUtils.exeCmd that matches "kill -0" command
         mockedOSUtils.when(() -> OSUtils.exeCmd(Mockito.matches(".*kill 
-0.*")))
                 .thenReturn("") // First invocation succeeds (process is alive)
@@ -176,10 +193,10 @@ public class ProcessUtilsTest {
         Assertions.assertTrue(result);
 
         // Verify SIGINT was called
-        mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -s SIGINT 12345"), 
Mockito.times(1));
+        mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -2 12345 1234"), 
Mockito.times(1));
         // Verify SIGTERM,SIGKILL was never called
-        mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -s SIGTERM 12345"), 
Mockito.never());
-        mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -s SIGKILL 12345"), 
Mockito.never());
+        mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -15 12345 1234"), 
Mockito.never());
+        mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -9 12345 1234"), 
Mockito.never());
     }
 
     @Test
@@ -190,27 +207,36 @@ public class ProcessUtilsTest {
         Mockito.when(taskRequest.getTenantCode()).thenReturn("testTenant");
 
         // Mock getPidsStr
-        mockedOSUtils.when(() -> 
OSUtils.exeCmd(Mockito.matches(".*pstree.*12345")))
-                .thenReturn("sudo(12345)---86.sh(1234)");
+        String pstreeCmd;
+        String pstreeOutput;
+        if (SystemUtils.IS_OS_MAC) {
+            pstreeCmd = "pstree -sp 12345";
+            pstreeOutput = "-+= 12345 sudo -+- 1234 86.sh";
+        } else {
+            pstreeCmd = "pstree -p 12345";
+            pstreeOutput = "sudo(12345)---86.sh(1234)";
+        }
+        mockedOSUtils.when(() -> 
OSUtils.exeCmd(pstreeCmd)).thenReturn(pstreeOutput);
 
         // Mock SIGINT command
-        mockedOSUtils.when(() -> OSUtils.getSudoCmd(Mockito.eq("testTenant"), 
Mockito.matches("kill -s SIGINT.*")))
-                .thenReturn("kill -s SIGINT 12345");
-        mockedOSUtils.when(() -> OSUtils.exeCmd("kill -s SIGINT 
12345")).thenReturn("");
+        mockedOSUtils.when(() -> OSUtils.getSudoCmd(Mockito.eq("testTenant"), 
Mockito.matches("kill -2.*")))
+                .thenReturn("kill -2 12345 1234");
+        mockedOSUtils.when(() -> OSUtils.exeCmd("kill -2 12345 
1234")).thenReturn("");
 
         // Mock SIGTERM command
-        mockedOSUtils.when(() -> OSUtils.getSudoCmd(Mockito.eq("testTenant"), 
Mockito.matches("kill -s SIGTERM.*")))
-                .thenReturn("kill -s SIGTERM 12345");
-        mockedOSUtils.when(() -> OSUtils.exeCmd("kill -s SIGTERM 
12345")).thenReturn("");
+        mockedOSUtils.when(() -> OSUtils.getSudoCmd(Mockito.eq("testTenant"), 
Mockito.matches("kill -15.*")))
+                .thenReturn("kill -15 12345 1234");
+        mockedOSUtils.when(() -> OSUtils.exeCmd("kill -15 12345 
1234")).thenReturn("");
 
         // Mock SIGKILL command
-        mockedOSUtils.when(() -> OSUtils.getSudoCmd(Mockito.eq("testTenant"), 
Mockito.matches("kill -s SIGKILL.*")))
-                .thenReturn("kill -s SIGKILL 12345");
-        mockedOSUtils.when(() -> OSUtils.exeCmd("kill -s SIGKILL 
12345")).thenReturn("");
+        mockedOSUtils.when(() -> OSUtils.getSudoCmd(Mockito.eq("testTenant"), 
Mockito.matches("kill -9.*")))
+                .thenReturn("kill -9 12345 1234");
+        mockedOSUtils.when(() -> OSUtils.exeCmd("kill -9 12345 
1234")).thenReturn("");
 
         // Mock kill -0
         mockedOSUtils.when(() -> OSUtils.getSudoCmd(Mockito.eq("testTenant"), 
Mockito.matches("kill -0.*")))
-                .thenReturn("kill -0 12345");
+                .thenReturn("kill -0 12345")
+                .thenReturn("kill -0 1234");
         mockedOSUtils.when(() -> OSUtils.exeCmd(Mockito.matches(".*kill 
-0.*"))).thenReturn("");
 
         // Act
@@ -220,9 +246,9 @@ public class ProcessUtilsTest {
         Assertions.assertFalse(result);
 
         // Verify SIGINT, SIGTERM, SIGKILL was called
-        mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -s SIGINT 12345"), 
Mockito.times(1));
-        mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -s SIGTERM 12345"), 
Mockito.times(1));
-        mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -s SIGKILL 12345"), 
Mockito.times(1));
+        mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -2 12345 1234"), 
Mockito.times(1));
+        mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -15 12345 1234"), 
Mockito.times(1));
+        mockedOSUtils.verify(() -> OSUtils.exeCmd("kill -9 12345 1234"), 
Mockito.times(1));
     }
 
     @Test

Reply via email to