janet-lang / spork

Various Janet utility modules - the official "Contrib" library.
MIT License
117 stars 35 forks source link

Do `exec-slurp` and `exec-slurp-all` benefit from tighter resource control? #173

Closed amano-kenji closed 7 months ago

amano-kenji commented 7 months ago

I wrote the following code because my janet fibers started crashing due to lack of pipes and files. I think garbage collector doesn't close pipes fast enough.

If pipes aren't closed, a long-running process will run out of pipes and files.

If I run the functions below in ev/with-deadline to prevent zombie processes from blocking janet, I still need to kill the zombie processes with os/proc-kill. Otherwise, I see zombie processes abandoned by janet.

(defn slurp
  ``
  It executes args with `os/spawn` and throws an error if the process returns with non-zero exit code.

  If the process exits with zero exit code, this functions returns standard output of the process.

  If there is any error, to prevent zombie processes, the spawned process is killed.

  Regardless of errors, the spawned process is going to be closed for resource control.
  ``
  [& args]
  (with [proc (os/spawn args :xp {:out :pipe})]
    (try
      (let [[out] (ev/gather
                    (ev/read (proc :out) :all)
                    (os/proc-wait proc))]
        out)
      ([err f]
        (try
          (os/proc-kill proc)
          ([_]))
        (propagate err f)))))

(defn slurp-all
  ``
  It executes args with `os/spawn` and returns a struct which has the following keys.

  * `:out` - the standard output of the process
  * `:err` - the standard error of the process
  * `:exit-code` - the exit code of the process`

  If there is any error, to prevent zombie processes, the spawned process is killed.

  Regardless of errors, the spawned process is going to be closed for resource control.
  ``
  [& args]
  (with [proc (os/spawn args :p {:out :pipe :err :pipe})]
    (try
      (let [[out err ec]
            (ev/gather
              (ev/read (proc :out) :all)
              (ev/read (proc :err) :all)
              (os/proc-wait proc))]
        {:out out :err err :exit-code ec})
      ([err f]
        (try
          (os/proc-kill proc)
          ([_]))
        (propagate err f)))))

I'm a bit worried about pipe buffer. If ev/with-deadline detaches the functions above from a zombie process, ev/read may not fully drain the pipe buffer. Does os/proc-kill drain pipe buffer?

amano-kenji commented 7 months ago
(def proc (os/spawn ["sh" "-c" "ls; sleep 1000"] :p {:out :pipe :err :pipe}))
(def {:out out :err err} proc)

(try
  (ev/with-deadline 5
    (let [[out err ec]
          (ev/gather
            (ev/read out :all)
            (ev/read err :all)
            (os/proc-wait proc))]
      (printf "stdout: %n" out)
      (printf "stderr: %n" err)
      (printf "exit code: %n" ec)))
  ([e f]
   (os/proc-kill proc)
   (eprintf "ev/read err :all = %n" (ev/read err :all))
   (eprintf "ev/read out :all = %n" (ev/read out :all))
   (propagate e f)))

prints

ev/read err :all = nil
ev/read out :all = nil
error: deadline expired
  in ev/take [src/core/ev.c] on line 983
  in <anonymous> [boot.janet] on line 3753, column 26
  in wait-for-fibers [boot.janet] on line 3751, column 5
  in <anonymous> [test.janet] on line 7, column 11
  in <anonymous> [test.janet] on line 5, column 3
  in _thunk [test.janet] (tailcall) on line 18, column 4
bakpakin commented 7 months ago

If you are worried about draining the pipe buffer in case of timeout, I recommend first setting up a buffer for stdout and stderr ahead of time, and then replacing (ev/read out :all) with (ev/read out :all my-buffer). That works for me to see the output of Ls even after timeout

amano-kenji commented 7 months ago

My looming theory is that there is no global pipe buffer. Each pipe has its own local buffer.

If I close the pipe, the pipe buffer goes away with the pipe. Thus, it should be sufficient to close the pipe or the spawned process because os/proc-close closes process pipes.

However, linux has a limit on the number of open file descriptors for each process. Thus, I should close pipes when I'm done. If I let garbage collector close pipes, the garbage collector may not close pipes fast enough.

amano-kenji commented 7 months ago

My estimation is that these functions will take care of zombie processes and any pipe issues properly.

(defn slurp
  ``
  It executes args with `os/spawn` and throws an error if the process returns with non-zero exit code.

  If the process exits with zero exit code, this functions returns standard output of the process.

  If there is any error, to prevent zombie processes, the spawned process is killed.

  Regardless of errors, the spawned process is going to be closed for resource control.
  ``
  [& args]
  # This closes proc along with its pipes. If pipes are not closed, janet will run out of file descriptors.
  (with [proc (os/spawn args :xp {:out :pipe})]
    (try
      (let [[out] (ev/gather
                    (ev/read (proc :out) :all)
                    (os/proc-wait proc))]
        out)
      ([err f]
        (try
           # If ev/with-deadline wraps this function and causes it to throw an error,
           # the spanwed process is probably a zombie process and should be killed.
          (os/proc-kill proc)
          ([_]))
        (propagate err f)))))

(defn slurp-all
  ``
  It executes args with `os/spawn` and returns a struct which has the following keys.

  * `:out` - the standard output of the process
  * `:err` - the standard error of the process
  * `:exit-code` - the exit code of the process`

  If there is any error, to prevent zombie processes, the spawned process is killed.

  Regardless of errors, the spawned process is going to be closed for resource control.
  ``
  [& args]
  # This closes proc along with its pipes. If pipes are not closed, janet will run out of file descriptors.
  (with [proc (os/spawn args :p {:out :pipe :err :pipe})]
    (try
      (let [[out err ec]
            (ev/gather
              (ev/read (proc :out) :all)
              (ev/read (proc :err) :all)
              (os/proc-wait proc))]
        {:out out :err err :exit-code ec})
      ([err f]
        (try
          # If ev/with-deadline wraps this function and causes it to throw an error,
          # the spanwed process is probably a zombie process and should be killed.
          (os/proc-kill proc)
          ([_]))
        (propagate err f)))))