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 6a82dc3d270081d073f9af09c12d516126036b49..a5b3f430ee09bf1883fce76224598ce2e20e78d7 100644 --- a/common/source/java/ch/systemsx/cisd/common/process/ProcessExecutionHelper.java +++ b/common/source/java/ch/systemsx/cisd/common/process/ProcessExecutionHelper.java @@ -68,15 +68,9 @@ public class ProcessExecutionHelper public static final OutputReadingStrategy DEFAULT_OUTPUT_READING_STRATEGY = OutputReadingStrategy.ON_ERROR; - /** Corresponds to no timeout at all for the process execution. */ - public static final long NO_TIMEOUT = ConcurrencyUtilities.NO_TIMEOUT; - /** Corresponds to a short timeout of 1/10 s. */ private static final long SHORT_TIMEOUT = 100; - /** Corresponds to an immediate timeout for the process execution. */ - private static final long IMMEDIATE_TIMEOUT = ConcurrencyUtilities.IMMEDIATE_TIMEOUT; - /** The executor service handling the threads that OS processes are spawned in. */ private static final ExecutorService executor = new NamingThreadPoolExecutor("osproc", 10); @@ -113,7 +107,7 @@ public class ProcessExecutionHelper public static boolean runAndLog(final List<String> cmd, final Logger operationLog, final Logger machineLog) throws StopException { - return new ProcessExecutionHelper(cmd, NO_TIMEOUT, DEFAULT_OUTPUT_READING_STRATEGY, + return new ProcessExecutionHelper(cmd, ConcurrencyUtilities.NO_TIMEOUT, DEFAULT_OUTPUT_READING_STRATEGY, operationLog, machineLog).runAndLog(); } @@ -129,7 +123,7 @@ public class ProcessExecutionHelper public static ProcessResult run(final List<String> cmd, final Logger operationLog, final Logger machineLog) throws StopException { - return new ProcessExecutionHelper(cmd, NO_TIMEOUT, DEFAULT_OUTPUT_READING_STRATEGY, + return new ProcessExecutionHelper(cmd, ConcurrencyUtilities.NO_TIMEOUT, DEFAULT_OUTPUT_READING_STRATEGY, operationLog, machineLog).run(true); } @@ -224,7 +218,7 @@ public class ProcessExecutionHelper /** * Returns the name of the command represented by <var>commandLine</var>. */ - static String getCommandName(final List<String> commandLine) + final static String getCommandName(final List<String> commandLine) { return new File(commandLine.get(0)).getName(); } @@ -232,7 +226,7 @@ public class ProcessExecutionHelper /** * Returns the command represented by <var>commandLine</var>. */ - private static String getCommand(final List<String> commandLine) + private final static String getCommand(final List<String> commandLine) { return StringUtils.join(commandLine, ' '); } @@ -282,7 +276,7 @@ public class ProcessExecutionHelper */ private class ProcessRunner implements NamedCallable<ProcessResult> { - private Process launch() throws IOException + private final Process launch() throws IOException { final ProcessBuilder processBuilder = new ProcessBuilder(commandLine); processBuilder.redirectErrorStream(true); @@ -290,11 +284,14 @@ public class ProcessExecutionHelper { operationLog.debug("Running command: " + getCommand(commandLine)); } - final Process process = processBuilder.start(); - return process; + return processBuilder.start(); } - public ProcessResult call() throws Exception + // + // NamedCallable + // + + public final ProcessResult call() throws Exception { try { @@ -341,9 +338,24 @@ public class ProcessExecutionHelper * separate thread because we have observed that, depending on the operating system and Java * version, processes can hang indefinitely on launching. */ - private class ProcessKiller implements NamedCallable<ProcessResult> + private final class ProcessKiller implements NamedCallable<ProcessResult> { - public ProcessResult call() + private final int getExitValue(final Process process) + { + try + { + return process.exitValue(); + } catch (final IllegalThreadStateException ex) + { + return ProcessResult.NO_EXIT_VALUE; + } + } + + // + // NamedCallable + // + + public final ProcessResult call() { final Process process = processWrapper.getAndSet(null); if (process != null) @@ -356,7 +368,7 @@ public class ProcessExecutionHelper process.destroy(); // Note: this also closes the I/O streams. if (machineLog.isInfoEnabled()) { - machineLog.info(String.format("Killed '" + getCommand(commandLine)) + "'."); + machineLog.info(String.format("Killed '%s'.", getCommand(commandLine))); } final int exitValue = getExitValue(process); return new ProcessResult(commandLine, processNumber, ExecutionStatus.TIMED_OUT, "", @@ -367,18 +379,7 @@ public class ProcessExecutionHelper } } - private int getExitValue(final Process process) - { - try - { - return process.exitValue(); - } catch (final IllegalThreadStateException ex) - { - return ProcessResult.NO_EXIT_VALUE; - } - } - - public String getCallableName() + public final String getCallableName() { return "kill-P" + processNumber + "-{" + getCommandName(commandLine) + "}"; } @@ -392,49 +393,45 @@ public class ProcessExecutionHelper this.processNumber = processCounter.getAndIncrement(); this.operationLog = operationLog; this.machineLog = machineLog; - // Backward compatibility. - if (millisToWaitForCompletion == IMMEDIATE_TIMEOUT) - { - this.millisToWaitForCompletion = NO_TIMEOUT; - } else - { - this.millisToWaitForCompletion = millisToWaitForCompletion; - } + this.millisToWaitForCompletion = millisToWaitForCompletion; this.outputReadingStrategy = outputReadingStrategy; this.commandLine = Collections.unmodifiableList(commandLine); this.processWrapper = new AtomicReference<Process>(); } - private ProcessResult run(final boolean stopOnInterrupt) + private final ProcessResult run(final boolean stopOnInterrupt) { final Future<ProcessResult> runnerFuture = executor.submit(new ProcessRunner()); - ExecutionResult<ProcessResult> result = + ExecutionResult<ProcessResult> processResult = ConcurrencyUtilities.getResult(runnerFuture, millisToWaitForCompletion, false, null, null); - if (result.getStatus() == ExecutionStatus.TIMED_OUT) + if (processResult.getStatus() == ExecutionStatus.TIMED_OUT) { final Future<ProcessResult> killerFuture = executor.submit(new ProcessKiller()); - result = ConcurrencyUtilities.getResult(killerFuture, SHORT_TIMEOUT); - if (result.tryGetResult() == null) + processResult = ConcurrencyUtilities.getResult(killerFuture, SHORT_TIMEOUT); + if (processResult.tryGetResult() == null) { - result = ConcurrencyUtilities.getResult(runnerFuture, IMMEDIATE_TIMEOUT); + processResult = + ConcurrencyUtilities.getResult(runnerFuture, + ConcurrencyUtilities.IMMEDIATE_TIMEOUT); } } - if (result.tryGetResult() != null) + final ProcessResult result = processResult.tryGetResult(); + if (result != null) { - return result.tryGetResult(); - } else if (stopOnInterrupt && ExecutionStatus.INTERRUPTED.equals(result.getStatus())) + return result; + } else if (stopOnInterrupt && ExecutionStatus.INTERRUPTED.equals(processResult.getStatus())) { throw new StopException(); } else { - return new ProcessResult(commandLine, processNumber, result.getStatus(), - tryGetStartupFailureMessage(result.tryGetException()), + return new ProcessResult(commandLine, processNumber, processResult.getStatus(), + tryGetStartupFailureMessage(processResult.tryGetException()), ProcessResult.NO_EXIT_VALUE, null, operationLog, machineLog); } } - private static String tryGetStartupFailureMessage(final Throwable throwableOrNull) + private final static String tryGetStartupFailureMessage(final Throwable throwableOrNull) { if (throwableOrNull != null && throwableOrNull instanceof IOException) { @@ -445,7 +442,7 @@ public class ProcessExecutionHelper } } - private boolean runAndLog() throws StopException + private final boolean runAndLog() throws StopException { final ProcessResult result = run(false); result.log(); diff --git a/common/source/java/ch/systemsx/cisd/common/utilities/FileUtilities.java b/common/source/java/ch/systemsx/cisd/common/utilities/FileUtilities.java index de445e36be6e5f2930caed4ec351b59668079401..63c315f9d3590df3b263383265e5a993b4473220 100644 --- a/common/source/java/ch/systemsx/cisd/common/utilities/FileUtilities.java +++ b/common/source/java/ch/systemsx/cisd/common/utilities/FileUtilities.java @@ -36,8 +36,6 @@ import java.text.NumberFormat; import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import java.util.concurrent.Semaphore; -import java.util.concurrent.TimeUnit; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -1013,53 +1011,6 @@ public final class FileUtilities } } - /** - * Returns <code>true</code>, if the (remote resource) <var>path</var> becomes available - * within <var>timeOutMillis</var> milli-seconds. - */ - public final static boolean isAvailable(final File path, final long timeOutMillis) - { - final Semaphore sem = new Semaphore(1); - sem.acquireUninterruptibly(); - final Thread t = new Thread(new Runnable() - { - public void run() - { - boolean pathExists = false; - do - { - pathExists = path.exists(); - if (pathExists) - { - sem.release(); - return; - } - try - { - Thread.sleep(timeOutMillis / 10L); - } catch (InterruptedException ex) - { - return; - } - } while (true); - } - }, "Path Availability Checker: " + path.getPath()); - t.start(); - try - { - final boolean exists = sem.tryAcquire(timeOutMillis, TimeUnit.MILLISECONDS); - if (exists == false) - { - t.interrupt(); - } - return exists; - } catch (final InterruptedException ex) - { - // This is not expected to happen. - return false; - } - } - private static final NumberFormat SIZE_FORMAT = new DecimalFormat("0.00"); /** 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 23b124803c8d79b7fd6c56663c99e48cc3ebaaad..f63ebd9caf6af3735fa573100b5b9f22458c2cbf 100644 --- a/common/sourceTest/java/ch/systemsx/cisd/common/process/ProcessExecutionHelperTest.java +++ b/common/sourceTest/java/ch/systemsx/cisd/common/process/ProcessExecutionHelperTest.java @@ -32,6 +32,7 @@ import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; import ch.systemsx.cisd.common.collections.CollectionIO; +import ch.systemsx.cisd.common.concurrent.ConcurrencyUtilities; import ch.systemsx.cisd.common.exceptions.StopException; import ch.systemsx.cisd.common.logging.LogCategory; import ch.systemsx.cisd.common.logging.LogFactory; @@ -247,7 +248,7 @@ public class ProcessExecutionHelperTest + stdout2, "echo " + stderr2); final ProcessResult result = ProcessExecutionHelper.run(Arrays.asList(dummyExec.getAbsolutePath()), - operationLog, machineLog, ProcessExecutionHelper.NO_TIMEOUT, + operationLog, machineLog, ConcurrencyUtilities.NO_TIMEOUT, OutputReadingStrategy.ALWAYS, false); final int exitValue = result.getExitValue(); assertEquals(0, exitValue); diff --git a/common/sourceTest/java/ch/systemsx/cisd/common/utilities/FileUtilitiesTest.java b/common/sourceTest/java/ch/systemsx/cisd/common/utilities/FileUtilitiesTest.java index e919d610b952e1171bcf98c04c5dd46b5ac8095d..f83938daf24be44db5530700866db9852b5e4662 100644 --- a/common/sourceTest/java/ch/systemsx/cisd/common/utilities/FileUtilitiesTest.java +++ b/common/sourceTest/java/ch/systemsx/cisd/common/utilities/FileUtilitiesTest.java @@ -30,8 +30,6 @@ import java.io.IOException; import java.io.InputStream; import java.util.Arrays; import java.util.List; -import java.util.Timer; -import java.util.TimerTask; import java.util.regex.Pattern; import org.apache.commons.io.FileUtils; @@ -398,54 +396,6 @@ public final class FileUtilitiesTest extends AbstractFileSystemTestCase } - @Test - public final void testIsAvailableFileHappyCase() throws IOException - { - final File f = new File(workingDirectory, "this_file_is_available"); - f.createNewFile(); - f.deleteOnExit(); - assertTrue(FileUtilities.isAvailable(f, 500L)); - } - - @Test - public final void testIsAvailableDirHappyCase() throws IOException - { - final File d = new File(workingDirectory, "this_dir_is_available"); - d.mkdir(); - d.deleteOnExit(); - assertTrue(FileUtilities.isAvailable(d, 500L)); - } - - @Test(groups = "slow") - public final void testIsAvailableDoesntExist() throws IOException - { - final File f = new File(workingDirectory, "this_file_is_unavailable"); - assertFalse(FileUtilities.isAvailable(f, 500L)); - } - - @Test(groups = "slow") - public final void testIsAvailableBecomesAvailableInTime() throws IOException - { - final File f = new File(workingDirectory, "this_file_will_become_available"); - f.deleteOnExit(); - Timer t = new Timer(); - t.schedule(new TimerTask() - { - @Override - public void run() - { - try - { - f.createNewFile(); - } catch (IOException ex) - { - throw new CheckedExceptionTunnel(ex); - } - } - }, 250L); - assertTrue(FileUtilities.isAvailable(f, 500L)); - } - private byte[] resourceToByteArray(String resourcename) { final InputStream is = FileUtilitiesTest.class.getResourceAsStream(resourcename);