FIX JS-2167 Ignore shell jobs return values when being killed

sos-berlin · Oct 15, 2024 · ab7db7d · ab7db7d
1 parent 742c3ca
commit ab7db7d
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 11 deletions.
diff --git a/js7-launcher/src/main/scala/js7/launcher/process/ProcessDriver.scala b/js7-launcher/src/main/scala/js7/launcher/process/ProcessDriver.scala
@@ -27,6 +27,7 @@ final class ProcessDriver(
   import jobLauncherConf.crashPidFile
   private val pipedProcessOnce = SetOnce[PipedProcess]
   private val startProcessLock = AsyncLock(orderId.toString)
+  @volatile private var killed = false
   @volatile private var killedBeforeStart: Option[ProcessSignal] = None
 
   def runProcess(env: Map[String, Option[String]], stdObservers: StdObservers)
@@ -108,16 +109,19 @@ final class ProcessDriver(
                     Checked.unit
 
     private def fetchReturnValues(returnCode: ReturnCode): IO[OrderOutcome.Completed] =
-      returnValuesProvider.read.catchAsChecked
-        .logWhenItTakesLonger(s"fetchReturnValues $orderId") // Because IO.interruptible does not execute ?
-        .map:
-          case Left(problem) =>
-            OrderOutcome.Failed.fromProblem(
-              problem.withPrefix("Reading return values failed:"),
-              Map(ProcessExecutable.toNamedValue(returnCode)))
-
-          case Right(namedValues) =>
-            conf.toOutcome(namedValues, returnCode)
+      if killed then
+        IO.pure(conf.toOutcome(NamedValues.empty, returnCode))
+      else
+        returnValuesProvider.read.catchAsChecked
+          .logWhenItTakesLonger(s"fetchReturnValues $orderId") // Because IO.interruptible does not execute ?
+          .map:
+            case Left(problem) =>
+              OrderOutcome.Failed.fromProblem(
+                problem.withPrefix("Reading return values file failed:"),
+                Map(ProcessExecutable.toNamedValue(returnCode)))
+
+            case Right(namedValues) =>
+              conf.toOutcome(namedValues, returnCode)
 
   def kill(signal: ProcessSignal): IO[Unit] =
     startProcessLock.lock("kill"):
@@ -131,7 +135,9 @@ final class ProcessDriver(
             sendProcessSignal(pipedProcess, signal)
 
   private def sendProcessSignal(pipedProcess: PipedProcess, signal: ProcessSignal): IO[Unit] =
-    pipedProcess.sendProcessSignal(signal)
+    IO.defer:
+      killed = true
+      pipedProcess.sendProcessSignal(signal)
 
   override def toString = s"ProcessDriver(${conf.jobKey})"
 

diff --git a/js7-tests/src/test/scala/js7/tests/CancelOrdersTest.scala b/js7-tests/src/test/scala/js7/tests/CancelOrdersTest.scala
@@ -991,12 +991,14 @@ final class CancelOrdersTest
                 if isWindows then
                   s"""@echo off
                      |echo Start
+                     |echo result=Hello >>"%JS7_RETURN_VALUES%"
                      |ping -n 11 127.0.0.1 >nul
                      |""".stripMargin
                 else
                   s"""#!/usr/bin/env bash
                      |set -euo pipefail
                      |echo Start
+                     |echo "result=Hello" >>"$$JS7_RETURN_VALUES"
                      |i=100
                      |while [ $$i -ge 0 ]; do sleep 0.1; done
                      |""".stripMargin),

diff --git a/js7-tests/src/test/scala/js7/tests/JobTimeoutTest.scala b/js7-tests/src/test/scala/js7/tests/JobTimeoutTest.scala
@@ -126,11 +126,13 @@ final class JobTimeoutTest extends OurTestSuite, ControllerAgentForScalaTest, Bl
               ShellScriptExecutable:
                 if isWindows then
                   s"""@echo off
+                     |echo result=Hello >>"%JS7_RETURN_VALUES%"
                      |ping -n ${jobDuration.toSeconds + 1} 127.0.0.1 >nul
                      |""".stripMargin
                 else
                   s"""#!/usr/bin/env bash
                      |set -euo pipefail
+                     |echo "result=Hello" >>"$$JS7_RETURN_VALUES"
                      |i=${10 * jobDuration.toSeconds}
                      |while [ $$i -ge 0 ]; do sleep 0.1; done
                      |""".stripMargin,