Commits

Michael Heemskerk  committed 2113dc7

STASH-3819 Fix thread safety issues in ExternalProcessImpl.isOutputPumpsRunning

A NPE could occur when a process is cancelled because the outputPump or
errorPump could be set to null by the thread calling cancel() while another
thread could be calling isOuputPumpRunning (as part of finish())

  • Participants
  • Parent commits 334b3bd

Comments (0)

Files changed (2)

File src/main/java/com/atlassian/utils/process/ExternalProcess.java

 
 public interface ExternalProcess extends Watchdog {
 
-    public ProcessHandler getHandler();
+    /**
+     * Get the process handler for this process execution
+     *
+     * @return the ProcessHandler instance associated with this process execution.
+     */
+    ProcessHandler getHandler();
 
-    public long getStartTime();
+    /**
+     * @return the time process execution started. -1 if the process has not yet started.
+     */
+    long getStartTime();
 
-    public void start();
+    /**
+     * Start the external process and setup the IO pump threads needed to
+     * manage the process IO. If you call this method you must eventually call the
+     * finish() method. Using this method you may execute additional code between process
+     * start and finish.
+     */
+    void start();
 
-    public void finish();
+    /**
+     * Finish process execution. This method should be called after you have called the {@link #start()} method.
+     */
+    void finish();
 
-    public boolean finish(int maxWait);
+    /**
+     * Wait a given time for the process to finish
+     *
+     * @param maxWait the maximum amount of time in milliseconds to wait for the process to finish
+     * @return true if the process has finished.
+     */
+    boolean finish(int maxWait);
 
-    public void execute();
+    /**
+     * Execute the external command. When this method returns, the process handler
+     * provided at construction time should be consulted to collect exit code, exceptions,
+     * process output, etc.
+     */
+    void execute();
 
-    public void executeWhile(Runnable runnable);
+    /**
+     * Executes the external command. While it is running, the given runnable is executed.
+     * The external command is not checked until the runnable completes
+     *
+     * @param runnable A task to perform while the external command is running.
+     */
+    void executeWhile(Runnable runnable);
 
-    public String getCommandLine();
+    /**
+     * @return the command line (command and arguments) executed by the process.
+     */
+    String getCommandLine();
 
     /**
      * @return true if the process is currently still running. {@code false} if the process has finished.
      */
-    public boolean isAlive();
+    boolean isAlive();
 }

File src/main/java/com/atlassian/utils/process/ExternalProcessImpl.java

 
     private final AtomicBoolean canceled = new AtomicBoolean(false);
     private final List<String> command;
+    private final ExecutorService executorService;
+    private final AtomicBoolean finished = new AtomicBoolean(false);
+    private final List<ProcessMonitor> monitors;
+
     private Map<String, String> environment;
+    private ProcessException processException;
+    private File workingDir;
+    private boolean suppressSpecialWindowsBehaviour;
+    private boolean useWindowsEncodingWorkaround;
+    private boolean useQuotesInBatArgumentsWorkaround;
+
     private volatile LatchedRunnable errorPump;
-    private final ExecutorService executorService;
     private volatile long executionTimeout;
-    private final AtomicBoolean finished = new AtomicBoolean(false);
     private volatile ProcessHandler handler;
     private volatile long idleTimeout;
     private volatile LatchedRunnable inputPump;
-    private volatile boolean inputPumpInterruptedAfterProcessFinished = false;
+    private volatile boolean inputPumpInterruptedAfterProcessFinished;
     private volatile long lastWatchdogReset;
-    private final List<ProcessMonitor> monitors;
     private volatile LatchedRunnable outputPump;
     private volatile Process process;
-    private ProcessException processException;
-    private boolean suppressSpecialWindowsBehaviour;
     private volatile long startTime;
-    private File workingDir;
-    private boolean useWindowsEncodingWorkaround;
-    private boolean useQuotesInBatArgumentsWorkaround;
 
     /**
      * Process an external command.
     }
 
     private boolean areOutputPumpsRunning() {
-        return (outputPump != null && outputPump.isRunning()) || (errorPump != null && errorPump.isRunning());
+        LatchedRunnable output = outputPump;
+        LatchedRunnable error = errorPump;
+
+        return (output != null && output.isRunning()) || (error != null && error.isRunning());
     }
 
     /**