diff --git a/common/source/java/ch/systemsx/cisd/common/process/ProcessExecutionHelper.java b/common/source/java/ch/systemsx/cisd/common/process/ProcessExecutionHelper.java index 5ba04cc04c882e290fee05cedc9afed3084158eb..b70e2194515386698810e697b5c84e2ea18fa2e9 100644 --- a/common/source/java/ch/systemsx/cisd/common/process/ProcessExecutionHelper.java +++ b/common/source/java/ch/systemsx/cisd/common/process/ProcessExecutionHelper.java @@ -332,7 +332,7 @@ public final class ProcessExecutionHelper { processWrapper.set(process); final int exitValue = process.waitFor(); - if (processWrapper.getAndSet(null) == null) + if (processWrapper.getAndSet(null) == null) { // Value is irrelevant, the ProcessKiller got us. return null; @@ -475,6 +475,9 @@ public final class ProcessExecutionHelper private ProcessResult getProcessResult(final boolean stopOnInterrupt, final Future<ProcessResult> runnerFuture) { + // when runUnblocking is used it is possible that we are hanging here while other thread + // runs the killer. We will get COMPLETE status and null as the ProcessResult. We have to + // change that status. ExecutionResult<ProcessResult> executionResult = getExecutionResult(runnerFuture); if (executionResult.getStatus() != ExecutionStatus.COMPLETE) { @@ -496,7 +499,13 @@ public final class ProcessExecutionHelper return result; } else { - return new ProcessResult(commandLine, processNumber, executionResult.getStatus(), + ExecutionStatus status = executionResult.getStatus(); + if (status == ExecutionStatus.COMPLETE) + { + // see the note above about termination from other thread + status = ExecutionStatus.INTERRUPTED; + } + return new ProcessResult(commandLine, processNumber, status, tryGetStartupFailureMessage(executionResult.tryGetException()), ProcessResult.NO_EXIT_VALUE, null, operationLog, machineLog); } diff --git a/common/sourceTest/java/ch/systemsx/cisd/common/process/ProcessExecutionHelperTest.java b/common/sourceTest/java/ch/systemsx/cisd/common/process/ProcessExecutionHelperTest.java index a81414c86c6d4c38c654a03f84802c718b2da099..aef08e4af7c06e5e6fcd3849d1aa2fcd825dab50 100644 --- a/common/sourceTest/java/ch/systemsx/cisd/common/process/ProcessExecutionHelperTest.java +++ b/common/sourceTest/java/ch/systemsx/cisd/common/process/ProcessExecutionHelperTest.java @@ -25,6 +25,7 @@ import java.io.IOException; import java.util.Arrays; import java.util.Timer; import java.util.TimerTask; +import java.util.concurrent.atomic.AtomicReference; import org.apache.log4j.Logger; import org.testng.annotations.BeforeClass; @@ -37,6 +38,7 @@ import ch.systemsx.cisd.common.exceptions.StopException; import ch.systemsx.cisd.common.logging.LogCategory; import ch.systemsx.cisd.common.logging.LogFactory; import ch.systemsx.cisd.common.logging.LogInitializer; +import ch.systemsx.cisd.common.process.ProcessExecutionHelper.IProcessHandler; import ch.systemsx.cisd.common.process.ProcessExecutionHelper.OutputReadingStrategy; /** @@ -273,4 +275,32 @@ public class ProcessExecutionHelperTest "some_non_existent_executable") >= 0); } + @Test(groups = + { "requires_unix", "slow" }) + public void testSleepyExecutionWithTermination() throws Exception + { + final File dummyExec = createSleepingExecutable("sleep.sh", 2 * WATCHDOG_WAIT_MILLIS); + final IProcessHandler handler = + ProcessExecutionHelper.runUnblocking(Arrays.asList(dummyExec.getAbsolutePath()), + operationLog, machineLog, WATCHDOG_WAIT_MILLIS); + final AtomicReference<ProcessResult> result = new AtomicReference<ProcessResult>(null); + final Runnable resultGetter = new Runnable() + { + public void run() + { + result.set(handler.getResult()); + } + }; + new Thread(resultGetter).start(); + Thread.sleep(WATCHDOG_WAIT_MILLIS / 2); + boolean terminated = handler.terminate(); + assertTrue(terminated); + Thread.sleep(WATCHDOG_WAIT_MILLIS / 20); + // Now resultGetter should be done with obtaining the result. + ProcessResult processResult = result.get(); + assertTrue(processResult != null); + assert processResult != null; // avoid compiler warnings + assertFalse(processResult.isOK()); // process terminated unsuccessfully + assertTrue(processResult.isInterruped() || processResult.isTerminated()); + } }