janet-lang / janet

A dynamic language and bytecode vm
https://janet-lang.org
MIT License
3.45k stars 223 forks source link

Add an option to os/pipe to create synchronous pipe #1481

Closed negrel closed 3 weeks ago

negrel commented 1 month ago

I'm currently building a shell DSL for Janet and I'm trying to pipe stdout of a process to stdin of another process.

I have the following code:

(def [pipe-r pipe-w] (os/pipe))

(pp (ev/gather
    (ev/spawn
        (def proc (os/spawn @["yes"] :epx { :out pipe-w }))
        (os/proc-wait proc)
        (ev/close pipe-w))
    (ev/spawn
        (def proc (os/spawn @["tr" "y" "Y"] :epx { :in pipe-r }))
        (os/proc-wait proc)
        (ev/close pipe-r))))

Here is the output:

$ janet main.janet |& uniq -c
      1 @[<fiber 0x000015E96C30> <fiber 0x000015E97140>]
      1 yes: standard output: Resource temporarily unavailable
  32768 Y
      1 tr: read error: Resource temporarily unavailable

The number of Y lines is always 32768 (2 << 14) or 0. After looking at the source code of Janet it seems that all pipe created using (os/pipe) are asynchronous (O_NONBLOCK).

The same code with a patched version of janet that returns synchronous pipe works well.

Does adding optional parameter to (os/pipe) to support synchronous pipe is conceivable ? Should I submit a PR ?

Context: https://janet.zulipchat.com/#narrow/stream/409517-help/topic/How.20to.20pipe.20output.20of.20process.20as.20input.20of.20another.20process.20.3F/near/456186185 This is related to https://github.com/janet-lang/janet/issues/592 I believe.

negrel commented 1 month ago

I'm not a windows developer but it seems that janet_make_pipe always return blocking pipe on windows, ignoring the mode argument.

bakpakin commented 1 month ago

Just pushed some changes to help address this by adding some extra options to os/pipe: https://github.com/janet-lang/janet/commit/accac6c6621bc7eb109aed50c8751c64ce0354f3

The following code should now work as both yes and tr should receive unmodified file descriptors and therefor avoid any strange behaviors.

(def [pipe-r pipe-w] (os/pipe :RW))

(pp (ev/gather
    (ev/spawn
        (def proc (os/spawn @["yes"] :epx { :out pipe-w }))
        (os/proc-wait proc)
                (ev/close pipe-w))
    (ev/spawn
        (def proc (os/spawn @["tr" "y" "Y"] :epx { :in pipe-r }))
        (os/proc-wait proc)
                (ev/close pipe-r))))
sogaiu commented 1 month ago

I saw a lot of Ys :)

(Warning: link to video)

negrel commented 1 month ago

It works well on my linux though it doesn't work on Windows because os_pipe calls janet_stream that calls janet_register_stream which call CreateIoCompletionPort with a handle that doesn't support overlapped IO.

bakpakin commented 3 weeks ago

This should now work on windows with some code to handle streams that don't support overlapped IO (they simply are not registered). Closing.