Re: [tarantool-patches] [PATCH] core: Non-blocking io.popen

[Vladimir Davydov <vdavydov.dev@gmail.com>]

[Cc += Alexander re popen API]

On Wed, Apr 24, 2019 at 08:00:32PM +0300, Stanislav Zudin wrote:
> Adds nonblocking implementation of popen.
> The method is available in namespace fio.
> fio.popen() returns an object with three methods:
> read(), write() and close().
> Method read() reads both STDOUT and STDERR output.
> 
> Closes #4031

Please write a DocBot request and be more thorough when describing the
new API: it should be clear what all the functions do and what arguments
they expect; there should be some examples. Currently it's unclear how
to use the new module judging by the description.

Anyway, the API doesn't look good enough to me:

 1. Reading both stdout and stderr with the same method doesn't make any
    sense. Those are two separate streams; each of them should have a
    separate file handle.
 2. There must be a way to ignore any of stdin/stdout/stderr. E.g. the
    caller might not be interested in stdout. However, if he doesn't
    read it, the program will just block once the pipe buffer has been
    filled. This is very inconvenient. I guess, we should use parent's
    stdout/stderr by default, but there also must be a way to completely
    silence the child's output (/dev/null).
 3. Is it really necessary to introduce separate read/write methods for
    popen? Can't we reuse existing fio.read/write? After all, a pipe can
    be used (almost) just like a normal file so why not simply wrap it
    in fio file handle?
 4. There must be a way to forcefully terminate a child program (kill).
 5. The caller should be able to obtain the exit code of a terminated
    child - without it it'd be impossible to figure out whether the
    program succeeded or failed.

I guess we have to go back to the drawing board and try to devise a good
API before proceeding to implementation. It may be worth to take a look
at other languages' versions of popen, e.g. Python's. Please try to come
up a good API and send out an RFC so that others can look at it and
comment.

Also, the test is insufficient: you should test all the functions
you introduce, all possible use cases. E.g. you don't check that
write(stderr) or read(stdin) works AFAICS.