[tarantool-patches] [PATCH] core: Non-blocking io.popen
Vladimir Davydov
vdavydov.dev at gmail.com
Tue May 7 11:49:49 MSK 2019
[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.
More information about the Tarantool-patches
mailing list