Skip to content

Commit

Permalink
Gracefully stop reading input in sub-process. Fixes #934.
Browse files Browse the repository at this point in the history
This commit handles three major bugs with the current way of redirecting input
from Leiningen to a sub-process:

- The redirection is never stopped. This means that typing on the keyboard after
  a sub-process has finished will crash the thread redirecting data from *in* to
  the sub-process' input stream, as it is closed.

- The redirection is buffered. This means that data intended to a subsequent
  sub-process may be sent to this sub-process instead.

- The redirection blocks instead of busy waits. For the output streams, this is
  perfectly fine and the recommended approach to avoid wasting cycles. However,
  this has some issues when done to an input stream: If the sub-process finishes
  while the thread block waiting for a character, we will be unable to stop the
  thread before a character has been read. The consequence is that a character
  originally intended to the subsequent sub-process will be given to the
  previous process.

These issues are solved by reading one byte at a time, busy wait on data and
gracefully exit when the sub-process has ended (through a mutable
variable/atom).
  • Loading branch information
hypirion committed Jan 18, 2013
1 parent 6e42cee commit 4f2bc32
Showing 1 changed file with 26 additions and 7 deletions.
33 changes: 26 additions & 7 deletions leiningen-core/src/leiningen/core/eval.clj
Original file line number Diff line number Diff line change
Expand Up @@ -121,15 +121,29 @@
(d-property [:http.proxyPort port])
(d-property [:http.nonProxyHosts non-proxy-hosts])]))))

(defn- pump [reader out]
(defn- out-pump [reader out]
(let [buffer (make-array Character/TYPE 1000)]
(loop [len (.read reader buffer)]
(when-not (neg? len)
(.write out buffer 0 len)
(.flush out)
(Thread/sleep 100)
(Thread/sleep 100) ;; TODO: Why is this here?
(recur (.read reader buffer))))))

(defn- in-pump
"Redirects input from this process to the input stream to the other process,
one byte at a time. Instead of blocking when reading, busy waits in order to
gracefully exit and not read other sub-processes' input."
[reader out done?]
(loop []
(if (.ready reader)
(do
(.write out (.read reader))
(.flush out))
(Thread/sleep 10))
(when (not @done?)
(recur))))

(def ^:dynamic *dir*
"Directory in which to start subprocesses with eval-in-project or sh."
(System/getProperty "user.dir"))
Expand Down Expand Up @@ -168,13 +182,18 @@
(with-open [out (io/reader (.getInputStream proc))
err (io/reader (.getErrorStream proc))
in (io/writer (.getOutputStream proc))]
(let [pump-out (doto (Thread. (bound-fn [] (pump out *out*))) .start)
pump-err (doto (Thread. (bound-fn [] (pump err *err*))) .start)
pump-in (Thread. (bound-fn [] (pump *in* in)))]
(let [done (atom false)
pump-out (doto (Thread. (bound-fn [] (out-pump out *out*))) .start)
pump-err (doto (Thread. (bound-fn [] (out-pump err *err*))) .start)
pump-in (Thread. (bound-fn [] (in-pump *in* in done)))]
(when *pump-in* (.start pump-in))
(.join pump-out)
(.join pump-err))
(.waitFor proc))))
(.join pump-err)
(let [exit-value (.waitFor proc)]
(when *pump-in*
(reset! done true)
(.join pump-in))
exit-value)))))

;; work around java's command line handling on windows
;; http://bit.ly/9c6biv This isn't perfect, but works for what's
Expand Down

0 comments on commit 4f2bc32

Please sign in to comment.