In last post Executing shell command in Clojure REPL. I present a function that can execute shell command from Clojure REPL.

Well, the script will work in most situations but you will find it not work in some edge cases because there are something missing in our implementation.

The reason is that Runtime.exec() is a very tricky method, if not using it well, it will bite you.

If you want details about Runtime.exec, this link gives detail explanation about this topic. But its quite long. I will just give an simple introduction here, and provide an improved version of shell function.

The waitFor function hangs forever

You can go to the last post to see the function or the copy I paste right here.

 
 
(defn shell [cmd arg] 
  (let [
         p (.exec (Runtime/getRuntime) (str cmd " " arg))
         ip (.getInputStream p)
         ipr ( java.io.InputStreamReader. ip)
         br (java.io.BufferedReader. ipr)
       ]
    (.waitFor p)
 
    (loop [str (.readLine br) output []]
      (if (nil? str)
        output
        (recur (.readLine br) (conj output str))
      )
    )
  )
)
 

If you run (shell "cat" "file.txt") and the size of file.txt is quite large. You will see no output. The reason is waitFor call is hanging forever.

How does this happen? The reason is the output stream has limited buffer size, if its full and we don't deal with it, the process will block, and it never terminate.

The waitFor returns only after the process terminated, thus it will hang forever in this case. Because the code below which will consume the buffer never get a chance to run.

This is what Javadoc says about it

Because some native platforms only provide limited buffer size for standard input and output streams, failure to promptly write the input stream or read the output stream of the subprocess may cause the subprocess to block, and even deadlock.

Not only waitFor will hang. The readLine also may hang.If we want to consume the error stream to prevent it from blocking the process before deal with the input stream.

 
(defn shell [cmd arg] 
  (let [
         p (.exec (Runtime/getRuntime) (str cmd " " arg))
         ip (.getInputStream p)
         ipr ( java.io.InputStreamReader. ip)
         br (java.io.BufferedReader. ipr)
         ip-error (.getErrorStream p)
         ipr-error ( java.io.InputStreamReader. ip-error)
         br-error (java.io.BufferedReader. ipr-error)
       ]
    ;; will hang
    (.readLine br-error)
 
    (loop [str (.readLine br) output []]
      (if (nil? str)
        (do (.waitFor p) output)
        (recur (.readLine br) (conj output str))
      )
    )
  )
)
 
 

If the input stream is full, the readLine from error stream will block. My final version looks like this

 
(defn shell [cmd arg] 
  (let [
         p (.exec (Runtime/getRuntime) (str cmd " " arg))
         ip (.getInputStream p)
         ipr ( java.io.InputStreamReader. ip)
         br (java.io.BufferedReader. ipr)
 
         ip-error (.getErrorStream p)
         ipr-error ( java.io.InputStreamReader. ip-error)
         br-error (java.io.BufferedReader. ipr-error)
       ]
 
    (let [ return-output 
      (loop [str (.readLine br) output []]
        (if (nil? str)
          output
          (recur (.readLine br) (conj output str))
        )
      )
        ]
 
      ;; consume error stream to prevent process from hanging
      (loop [str (.readLine br-error)]        
        (if (nil? str)
          (do (.waitFor p) "")
          (recur (.readLine br-error))
        )
      )
      return-output
    )
  )
)
 

This version will first consume all data in input stream and then consume error stream, then waitFor the process terminate and finally return the output from input stream.

What if the error stream is full, will it make the readLine calling on input stream hang? I don't know, I can't even find an example with large enough error output to test it. According to this quote from link

A better solution would empty both the standard error stream and the standard output stream. And the best solution would empty these streams simultaneously

This must be true. But do this will make our code more complex, we may have to do them in two threads. So for me, this version is good enough.