[Tarantool-patches] [PATCH 0/5] popen: Add ability to run external process

[Tarantool-patches] [PATCH 0/5] popen: Add ability to run external process

[Cyrill Gorcunov]

In this series we provide a way to execute external binaries
from inside of Lua scripts, control children process and
communicate with their stdin/out/err streams.

The series is preliminary since there are known issues
I have to resolve before merging. Still to share code
early I decided to post them even in such shape.

While I'm more-less sure about stability of the engine
itself (which lays in src/lib/core/popen.c) the lua interface
which sits in src/lua/fio.lua looks not that convenient I think.

All created popen objects require explicit popen:close call
probably I should hook up lua GC (as I being advised).

Anyway please take a look once time permit. Any comments
are highly appreciated.

All patches are sitting in branch gorcunov/gh-4031-popen

Cyrill Gorcunov (5):
  popen: Introduce a backend engine
  lua/fio: Add lbox_fio_push_error as a separate helper
  popen/fio: Merge popen engine into fio internal module
  popen/fio: Implement lua interface for a popen object
  test: Add app/popen test

 src/lib/core/CMakeLists.txt |    1 +
 src/lib/core/coio_file.c    |  155 +++++
 src/lib/core/coio_file.h    |   10 +
 src/lib/core/popen.c        | 1204 +++++++++++++++++++++++++++++++++++
 src/lib/core/popen.h        |  135 ++++
 src/lua/fio.c               |  250 +++++++-
 src/lua/fio.lua             |  434 +++++++++++++
 src/main.cc                 |    4 +
 test/app/popen.result       |  105 +++
 test/app/popen.test.lua     |   39 ++
 10 files changed, 2333 insertions(+), 4 deletions(-)
 create mode 100644 src/lib/core/popen.c
 create mode 100644 src/lib/core/popen.h
 create mode 100644 test/app/popen.result
 create mode 100644 test/app/popen.test.lua

-- 
2.20.1

[Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine

[Cyrill Gorcunov]

In the patch we introduce popen backend engine which provides
a way to execute external programs and communicate with their
stdin/stdout/stderr streams.

It is possible to run a child process with:

 a) completely closed stdX descriptors
 b) provide /dev/null descriptors to appropritate stdX
 c) pass new transport into a child (currently we use
    pipes for this sake, but mae extend to tty/sockets)
 d) inherit stdX from a parent, iow do nothing

On tarantool start we create @popen_pids_map hash which maps
created processes PIDs to popen_handle structure, this structure
keeps everything needed to control and communicate with the children.
The hash will allow us to find a hild process quickly from inside
of a signal handler.

Each handle links into @popen_head list, which is need to be able
to destory children processes on exit procedure (ie when we exit
tarantool and need to cleanup the resources used).

Every new process is born by vfork call - we can't use fork()
because of at_fork handlers in libeio which cause deadlocking
in internal mutex usage. Thus the caller waits until vfork finishes
its work and runs exec (or exit with error). The good news is that
we are supposed to create new processes from coio separate thread
thus other threads are not affected.

Because children processes are running without any limitations
they can exit by self or can be killed by some other third side
(say user of a hw node), we need to watch their state which is
done by setting a hook with ev_child_start helper. This helper
allows us to catch SIGCHLD when a child get exited/signaled
and unregister it from a pool or currently running children.
Note the libev wait() reaps child zomby by self. Such specifics
force us to use signal blocking procedure as a sync point to keep
children pids consistent (without signal blocking pids could be
simply reused inbetween and we can't distinguish the pid we're
tracking belongs our child process or some other process in
the system).

This engine provides the following API:
 - popen_create -- to create a new child process
 - popen_destroy -- to release resources occupied and
   terminate a chile process
 - popen_kill -- to kill a child process, note that this
   routine doesn't wait for process termination it simply
   sends SIGKILL signal
 - popen_wstatus -- to fetch current system dependant status
   of a child process
 - popen_stat -- to fetch statistic of a child process

Known issues:

 - environment variables are flushed to zero, should we provide
   a way to adjust it (via options) or inherit it instead?

 - popen_kill always send SIGKILL, should not we provide a
   portable way to customize signal sedning (say symbolic
   names for signals and pass them here)?

 - for native mode we don't do additional processing of arguments
   thus only plain name of elf executable will be working, we
   should provide a way for argv explicit passing or
   do analyze @command for arguments by hands;

 - for linux base systems we use popen2 system call passing
   O_CLOEXEC flag so that two concurrent popen_create calls
   would not affect each other with pipes inheritance (while
   currently we don't have a case where concurrent calls could
   be done as far as I know, still better to be on a safe side
   from the beginning);

 - there are some files (such as xlog) which tarantool opens
   for own needs without setting O_CLOEXEC flag and it get
   propagated to a children process; for linux based systems
   we use close_inherited_fds helper which walks over opened
   files of a process and close them but for other targets
   like MachO or FreeBSD this helper just zapped simply because
   I don't have such machines to experimant with; we should
   investigate this moment in more details later once base
   code is merged in;

 - need to consider a case where we will be using piping for
   descriptors (for example we might be writting into stdin
   of a child from another pipe, for this sake we could use
   splice() syscall which gonna be a way faster than copying
   data inside kernel between process). Still the question
   is -- do we really need it? Since we use interanal flags
   in popen handle this should not be a big problem to extend
   this interfaces.

Part of #4031

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/lib/core/CMakeLists.txt |    1 +
 src/lib/core/popen.c        | 1204 +++++++++++++++++++++++++++++++++++
 src/lib/core/popen.h        |  135 ++++
 src/main.cc                 |    4 +
 4 files changed, 1344 insertions(+)
 create mode 100644 src/lib/core/popen.c
 create mode 100644 src/lib/core/popen.h

diff --git a/src/lib/core/CMakeLists.txt b/src/lib/core/CMakeLists.txt
index e60b5199e..0f080061e 100644
--- a/src/lib/core/CMakeLists.txt
+++ b/src/lib/core/CMakeLists.txt
@@ -15,6 +15,7 @@ set(core_sources
     coio.cc
     coio_task.c
     coio_file.c
+    popen.c
     coio_buf.cc
     fio.c
     exception.cc
diff --git a/src/lib/core/popen.c b/src/lib/core/popen.c
new file mode 100644
index 000000000..460d3746c
--- /dev/null
+++ b/src/lib/core/popen.c
@@ -0,0 +1,1204 @@
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <paths.h>
+#include <signal.h>
+#include <poll.h>
+
+#include <sys/types.h>
+#include <dirent.h>
+#include <sys/wait.h>
+
+#include "popen.h"
+#include "assoc.h"
+#include "say.h"
+
+/* Children pids map popen_handle map */
+static struct mh_i32ptr_t *popen_pids_map = NULL;
+
+/* All popen handles to be able to cleanup them on exit */
+static RLIST_HEAD(popen_head);
+
+/* /dev/null to be used inside children if requested */
+static int dev_null_fd_ro = -1;
+static int dev_null_fd_wr = -1;
+
+/* To block SIGCHLD delivery when need a sync point */
+static sigset_t popen_blockmask;
+
+/*
+ * In case if something really unexpected happened
+ * and we no longer able to unblock SIGCHLD instead
+ * of exiting with error in a middle of program work
+ * we rather disable new popen openings leaving a user
+ * a way to shutdown without loosing a memory data.
+ */
+static bool popen_blockmask_broken = false;
+
+/**
+ * popen_register - register popen handle in a pids map
+ * @handle: a handle to register
+ */
+static void
+popen_register(struct popen_handle *handle)
+{
+	struct mh_i32ptr_node_t node = {
+		.key	= handle->pid,
+		.val	= handle,
+	};
+	say_debug("popen: register %d", handle->pid);
+	mh_i32ptr_put(popen_pids_map, &node, NULL, NULL);
+}
+
+/**
+ * popen_find - find popen handler by its pid
+ * @pid: pid of a handler
+ *
+ * Returns a handle if found or NULL otherwise.
+ */
+static struct popen_handle *
+popen_find(pid_t pid)
+{
+	mh_int_t k = mh_i32ptr_find(popen_pids_map, pid, NULL);
+	if (k == mh_end(popen_pids_map))
+		return NULL;
+	return mh_i32ptr_node(popen_pids_map, k)->val;
+}
+
+/**
+ * popen_unregister - remove popen handler from a pids map
+ * @handle: a handle to remove
+ */
+static void
+popen_unregister(struct popen_handle *handle)
+{
+	struct mh_i32ptr_node_t node = {
+		.key	= handle->pid,
+		.val	= NULL,
+	};
+	say_debug("popen: unregister %d", handle->pid);
+	mh_i32ptr_remove(popen_pids_map, &node, NULL);
+}
+
+/**
+ * handle_alloc - allocate new popen hanldle with flags specified
+ * @flags: flags to be used
+ *
+ * Everything else initialized to default values.
+ *
+ * Returns pointer to a new popen or NULL on error.
+ */
+static struct popen_handle *
+handle_alloc(unsigned int flags)
+{
+	struct popen_handle *handle;
+
+	handle = malloc(sizeof(*handle));
+	if (!handle) {
+		say_syserror("popen: Can't allocate handle");
+		return NULL;
+	}
+
+	handle->wstatus	= 0;
+	handle->pid	= -1;
+	handle->flags	= flags;
+
+	rlist_create(&handle->list);
+
+	/* all fds to -1 */
+	memset(handle->fds, 0xff, sizeof(handle->fds));
+
+	say_debug("popen: allocated %p", handle);
+	return handle;
+}
+
+/**
+ * handle_free - free memory allocated for a handle
+ * @handle: a handle to free
+ *
+ * Just to match handle_alloc().
+ */
+static void
+handle_free(struct popen_handle *handle)
+{
+	say_debug("popen: handle %p free %p", handle);
+	free(handle);
+}
+
+/**
+ * popen_may_io - test if handle can run io operation
+ * @handle:	popen handle
+ * @idx:	index of a file descriptor to operate on
+ * @io_flags:	popen_flag_bits flags
+ *
+ * Returns true if IO is allowed and false otherwise
+ * (setting an appropriate @errno).
+ */
+static inline bool
+popen_may_io(struct popen_handle *handle, unsigned int idx,
+	     unsigned int io_flags)
+{
+	if (!handle) {
+		errno = ESRCH;
+		return false;
+	}
+
+	if (!(io_flags & handle->flags)) {
+		errno = EINVAL;
+		return false;
+	}
+
+	if (handle->fds[idx] < 0) {
+		errno = EPIPE;
+		return false;
+	}
+	return true;
+}
+
+/**
+ * popen_may_pidop - test if handle is valid for pid related operations
+ * @handle:	popen handle
+ *
+ * This is shortcut to test if handle is not nil and still have
+ * a valid child process.
+ *
+ * Returns true if ops are allowed and false otherwise
+ * (setting an appropriate @errno).
+ */
+static inline bool
+popen_may_pidop(struct popen_handle *handle)
+{
+	if (!handle || handle->pid == -1) {
+		errno = ESRCH;
+		return false;
+	}
+	return true;
+}
+
+/**
+ * popen_stat - fill popen object statistics
+ * @handle:	popen handle
+ * @st:		destination popen_stat to fill
+ *
+ * Returns 0 on succes, -1 otherwise.
+ */
+int
+popen_stat(struct popen_handle *handle, struct popen_stat *st)
+{
+	if (!handle) {
+		errno = ESRCH;
+		return -1;
+	}
+
+	st->pid		= handle->pid;
+	st->flags	= handle->flags;
+
+	static_assert(lengthof(st->fds) == lengthof(st->fds),
+		      "Statistics fds are screwed");
+
+	memcpy(st->fds, handle->fds, sizeof(handle->fds));
+
+	return 0;
+}
+
+/**
+ * popen_write - write data to the child stdin
+ * @handle:	popen handle
+ * @buf:	data to write
+ * @count:	number of bytes to write
+ * @flags:	a flag representing stdin peer
+ *
+ * Returns number of bytes written or -1 on error.
+ */
+ssize_t
+popen_write(struct popen_handle *handle, void *buf,
+	    size_t count, unsigned int flags)
+{
+	if (!popen_may_io(handle, STDIN_FILENO, flags))
+		return -1;
+
+	say_debug("popen: %d: write idx %d",
+		  handle->pid, STDIN_FILENO);
+
+	return write(handle->fds[STDIN_FILENO], buf, count);
+}
+
+/**
+ * popen_wait_read - wait for data appear on a child's peer
+ * @handle:		popen handle
+ * @fd:			peer fd to wait on
+ * @timeout_msecs:	timeout in microseconds
+ *
+ * Returns 1 if there is data to read, -EAGAIN if timeout happened
+ * and -1 on other errors setting errno accordingly.
+ */
+static int
+popen_wait_read(struct popen_handle *handle, int fd, int timeout_msecs)
+{
+	struct pollfd pollfd = {
+		.fd	= fd,
+		.events	= POLLIN,
+	};
+	int ret;
+
+	ret = poll(&pollfd, 1, timeout_msecs);
+	say_debug("popen: %d: poll: ret %d fd %d events %#x revents %#x",
+		  handle->pid, ret, fd, pollfd.events, pollfd.revents);
+
+	if (ret == 1) {
+		if (pollfd.revents == POLLIN) {
+			return 1;
+		} else {
+			say_error("popen: %d: unexpected revents %#x",
+				  handle->pid, pollfd.revents);
+			return -EINVAL;
+		}
+	}
+
+	return ret < 0 ? -errno : -EAGAIN;
+}
+
+/**
+ * popen_read_timeout - read data from a child's peer with timeout
+ * @handle:		popen handle
+ * @buf:		destination buffer
+ * @count:		number of bytes to read
+ * @flags:		POPEN_FLAG_FD_STDOUT or POPEN_FLAG_FD_STDERR
+ * @timeout_msecs:	time to wait in microseconds if no
+ * 			data available; ignored if less or equal to zero
+ *
+ * Returns number of bytes read or -EAGAIN if @timeout_msecs expired.
+ * On other errors -1 returned and errno set accordingly.
+ */
+ssize_t
+popen_read_timeout(struct popen_handle *handle, void *buf,
+		   size_t count, unsigned int flags,
+		   int timeout_msecs)
+{
+	int idx, ret;
+
+	idx = flags & POPEN_FLAG_FD_STDOUT ?
+		STDOUT_FILENO : STDERR_FILENO;
+
+	if (!popen_may_io(handle, idx, flags))
+		return -1;
+
+	say_debug("popen: %d: read idx %d fds %d timeout_msecs %d",
+		  handle->pid, idx, handle->fds[idx], timeout_msecs);
+
+	if (timeout_msecs > 0) {
+		ret = popen_wait_read(handle, handle->fds[idx],
+				      timeout_msecs);
+		if (ret < 0) {
+			if (ret != -EAGAIN) {
+				errno = -ret;
+				say_syserror("popen: %d: data wait failed",
+					     handle->pid);
+			}
+			return ret;
+		}
+	}
+
+	return read(handle->fds[idx], buf, count);
+}
+
+/**
+ * wstatus_str - encode signal status into human readable form
+ * @buf:	destination buffer
+ * @size:	buffer size
+ * @wstatus:	status to encode
+ *
+ * Operates on S_DEBUG level only simply because snprintf
+ * is pretty heavy in performance.
+ *
+ * Returns pointer to @buf with encoded string.
+ */
+static char *
+wstatus_str(char *buf, size_t size, int wstatus)
+{
+	static const char fmt[] =
+		"wstatus %#x exited %d status %d "
+		"signaled %d wtermsig %d "
+		"stopped %d stopsig %d "
+		"coredump %d continued %d";
+
+	assert(size > 0);
+
+	if (say_log_level_is_enabled(S_DEBUG)) {
+		snprintf(buf, size, fmt, wstatus,
+			 WIFEXITED(wstatus),
+			 WIFEXITED(wstatus) ?
+			 WEXITSTATUS(wstatus) : -1,
+			 WIFSIGNALED(wstatus),
+			 WIFSIGNALED(wstatus) ?
+			 WTERMSIG(wstatus) : -1,
+			 WIFSTOPPED(wstatus),
+			 WIFSTOPPED(wstatus) ?
+			 WSTOPSIG(wstatus) : -1,
+			 WCOREDUMP(wstatus),
+			 WIFCONTINUED(wstatus));
+	}
+
+	return buf;
+}
+
+/**
+ * __wstatus_str - shortcut to wstatus_str with static buffer
+ * @wstatus:	status to encode
+ *
+ * Returns pointer to a buffer with encoded message.
+ * Note this function uses the local static buffer thus
+ * should not be called in parallel.
+ */
+static char *
+__wstatus_str(int wstatus)
+{
+	static char buf[128];
+	return wstatus_str(buf, sizeof(buf), wstatus);
+}
+
+/**
+ * popen_notify_sigchld - notify popen subsisteb about SIGCHLD event
+ * @pid:	PID of a process which changed its state
+ * @wstatus:	signal status of a process
+ *
+ * The function is called from global SIGCHLD watcher in libev so
+ * we need to figure out if it is our process which possibly been
+ * terminated.
+ *
+ * Note the libev calls for wait() by self so we don't need to do
+ * furter processing and reap children.
+ */
+static void
+popen_notify_sigchld(pid_t pid, int wstatus)
+{
+	struct popen_handle *handle;
+	static char buf[128];
+
+	say_debug("popen: sigchld notify %d (%s)",
+		  pid, wstatus_str(buf, sizeof(buf), wstatus));
+
+	handle = popen_find(pid);
+	if (!handle)
+		return;
+
+	handle->wstatus = wstatus;
+	if (WIFEXITED(wstatus) || WIFSIGNALED(wstatus)) {
+		assert(handle->pid == pid);
+		/*
+		 * libev calls for waitpid by self so
+		 * we don't have to wait here.
+		 */
+		popen_unregister(handle);
+		/*
+		 * Since SIGCHLD may come to us not
+		 * due to exit/kill reason (consider
+		 * a case when someone stopped a child
+		 * process) we should continue wathcing
+		 * state changes, thus we stop monitoring
+		 * dead children only.
+		 */
+		say_debug("popen: ev_child_stop %d", handle->pid);
+		ev_child_stop(EV_DEFAULT_ &handle->ev_sigchld);
+		handle->pid = -1;
+	}
+}
+
+/**
+ * ev_sigchld_cb - handle SIGCHLD from a child process.
+ * @w:		a child exited
+ * @revents:	unused
+ */
+static void
+ev_sigchld_cb(EV_P_ ev_child *w, int revents)
+{
+	(void)revents;
+	ev_child_stop(EV_A_ w);
+
+	/*
+	 * The reason for a separate helper is that
+	 * we might need to notify more subsystems
+	 * in future.
+	 */
+	popen_notify_sigchld(w->rpid, w->rstatus);
+}
+
+/**
+ * popen_sigchld_block - block SIGCHLD
+ * @oldmask:	a pointer where to save an old signal mask
+ *
+ * This routine is serialization point, we use signal blocking
+ * to prevent concurrent access to popen handle from external
+ * users which may kill programs by hands in any moment.
+ *
+ * Returns 0 on success, -1 otherwise.
+ */
+static int
+popen_sigchld_block(sigset_t *oldmask)
+{
+	if (unlikely(popen_blockmask_broken)) {
+		return 0;
+	} else if (sigprocmask(SIG_BLOCK, &popen_blockmask, oldmask)) {
+		say_syserror("popen: Can't block SIGCHLD");
+		return -1;
+	}
+	return 0;
+}
+
+/**
+ * popen_sigchld_block - unblock SIGCHLD
+ * @oldmask:	a pointer to a mask to restore
+ *
+ * Returns 0 on success, -1 otherwise.
+ */
+static int
+popen_sigchld_unblock(sigset_t *oldmask)
+{
+	if (unlikely(popen_blockmask_broken)) {
+		return 0;
+	} else if (sigprocmask(SIG_SETMASK, oldmask, NULL)) {
+		say_syserror("popen: Can't unblock SIGCHLD");
+		/*
+		 * This is critial issue but give users
+		 * an opportunity to shutdown.
+		 */
+		say_crit("popen: Signal handling is broken, "
+			 "please consider restarting the program.");
+		popen_blockmask_broken = true;
+		return -1;
+	}
+	return 0;
+}
+
+/**
+ * popen_wstatus_blocked - fetch popen child process wait status
+ * @handle:	popen handle to inspect
+ * @wstatus:	status to be filled if process exited
+ * @early:	early bootstrap testing, don't print error if true
+ *
+ * The SIGCHLD must be blocked.
+ *
+ * Returns 1 if process changed its state filling
+ * optional @wstatus, 0 if process is still running
+ * and -1 on error.
+ */
+static inline int
+popen_wstatus_blocked(struct popen_handle *handle,
+		      int *wstatus, bool early)
+{
+	int pid;
+
+	if (!popen_may_pidop(handle)) {
+		if (handle && wstatus)
+			*wstatus = handle->wstatus;
+		/*
+		 * Here is a trick if @handle is passed
+		 * and its pid = -1 it means we already
+		 * obtained sigchld so caller is interested
+		 * in child status way after the child is
+		 * finished.
+		 */
+		return handle ? 1 : -1;
+	}
+
+	pid = waitpid(handle->pid, &handle->wstatus, WNOHANG);
+	if (pid == -1 && !early) {
+		say_syserror("popen: Unable to wait pid %d (%s)",
+			     handle->pid, __wstatus_str(handle->wstatus));
+	} else if (pid > 0) {
+		if (wstatus)
+			*wstatus = handle->wstatus;
+		pid = 1;
+	}
+
+	return pid;
+}
+
+
+/**
+ * popen_wstatus - fetch popen child process wait status
+ * @handle:	popen handle to inspect
+ * @wstatus:	status to be filled if process exited
+ *
+ * Returns 1 if process changed its state filling
+ * optional @wstatus, 0 if process is still running
+ * and -1 on error.
+ */
+int
+popen_wstatus(struct popen_handle *handle, int *wstatus)
+{
+	sigset_t oldmask;
+	int ret;
+
+	/*
+	 * The pid in handle might be already killed
+	 * by external signal or via natural exit of
+	 * a program, so need to block.
+	 */
+	if (popen_sigchld_block(&oldmask))
+		return -1;
+	ret = popen_wstatus_blocked(handle, wstatus, false);
+	popen_sigchld_unblock(&oldmask);
+
+	return ret;
+}
+
+/**
+ * popen_kill_blocked - kills a child process with signals blocked
+ * @handle:	popen handle
+ *
+ * The SIGCHLD must be blocked.
+ *
+ * Returns 0 if child has been killed, -1 otherwise.
+ */
+static inline int
+popen_kill_blocked(struct popen_handle *handle)
+{
+	int ret;
+
+	/*
+	 * A child may be killed or exited already.
+	 */
+	if (popen_may_pidop(handle)) {
+		say_debug("popen: killpg %d", handle->pid);
+		ret = killpg(handle->pid, SIGKILL);
+		if (ret < 0) {
+			say_syserror("popen: Unable to kill %d",
+				     handle->pid);
+		}
+	} else
+		ret = -1;
+
+	return ret;
+}
+
+/**
+ * popen_kill - kills a child process associated with popen handle
+ * @handle:	popen handle
+ *
+ * Returns 0 if child has been killed, -1 otherwise.
+ */
+int
+popen_kill(struct popen_handle *handle)
+{
+	sigset_t oldmask;
+	int ret;
+
+	if (popen_sigchld_block(&oldmask))
+		return -1;
+	ret = popen_kill_blocked(handle);
+
+	popen_sigchld_unblock(&oldmask);
+	return ret;
+}
+
+/**
+ * popen_destroy_blocked - destory a popen handle
+ * @handle:	a popen handle to destroy
+ *
+ * The function kills a child process and
+ * close all fds and remove the handle from
+ * a living list and finally frees the handle.
+ *
+ * The SIGCHLD must be blocked or the handle
+ * must be not registered yet.
+ *
+ * Returns 0 on success, -1 otherwise.
+ */
+static inline int
+popen_destroy_blocked(struct popen_handle *handle)
+{
+	size_t i;
+
+	if (popen_kill(handle) && errno != ESRCH)
+		return -1;
+
+	for (i = 0; i < lengthof(handle->fds); i++) {
+		if (handle->fds[i] != -1)
+			close(handle->fds[i]);
+	}
+
+	/*
+	 * We won't be wathcing this child anymore if
+	 * kill signal is not yet delivered.
+	 */
+	if (handle->pid != -1) {
+		say_debug("popen: ev_child_stop %d", handle->pid);
+		ev_child_stop(EV_DEFAULT_ &handle->ev_sigchld);
+	}
+
+	rlist_del(&handle->list);
+	handle_free(handle);
+	return 0;
+}
+
+/**
+ * popen_destroy - destory a popen handle
+ * @handle:	pointer to a popen handle
+ *
+ * The function kills a child process associated with the
+ * popen handle, closes all pipes and frees memory.
+ *
+ * After this call the popen object no longer usable.
+ *
+ * Returns 0 on succsess, -1 otherwise.
+ */
+int
+popen_destroy(struct popen_handle *handle)
+{
+	sigset_t oldmask;
+	int ret;
+
+	if (!handle) {
+		errno = ESRCH;
+		return -1;
+	}
+
+	if (popen_sigchld_block(&oldmask))
+		return -1;
+	ret = popen_destroy_blocked(handle);
+	popen_sigchld_unblock(&oldmask);
+	return ret;
+}
+
+/**
+ * create_pipe - create nonblocking cloexec pipe
+ * @pfd:	pipe ends to setup
+ *
+ * Returns 0 on success, -1 on error.
+ */
+static int
+create_pipe(int pfd[2])
+{
+#ifdef TARGET_OS_LINUX
+	if (pipe2(pfd, O_CLOEXEC)) {
+		say_syserror("popen: Can't create pipe2");
+		return -1;
+	}
+#else
+	if (pipe(pfd)) {
+		say_syserror("popen: Can't create pipe");
+		return -1;
+	}
+	if (fcntl(pfd[0], F_SETFL, O_CLOEXEC) ||
+	    fcntl(pfd[1], F_SETFL, O_CLOEXEC)) {
+		int saved_errno = errno;
+		say_syserror("popen: Can't unblock pipe");
+		close(pfd[0]), pfd[0] = -1;
+		close(pfd[1]), pfd[1] = -1;
+		errno = saved_errno;
+		return -1;
+	}
+#endif
+	return 0;
+}
+
+/**
+ * close_inherited_fds - close inherited file descriptors
+ * @skip_fds:		an array of descriptors which should
+ * 			be kept opened
+ * @nr_skip_fds:	number of elements in @skip_fds
+ *
+ * Returns 0 on success, -1 otherwise.
+ */
+static int
+close_inherited_fds(int *skip_fds, size_t nr_skip_fds)
+{
+#ifdef TARGET_OS_LINUX
+	static const char path[] = "/proc/self/fd";
+	struct dirent *de;
+	int fd_no, fd_dir;
+	DIR *dir;
+	size_t i;
+
+	dir = opendir(path);
+	if (!dir) {
+		say_syserror("popen: fdin: Can't open %s", path);
+		return -1;
+	}
+	fd_dir = dirfd(dir);
+
+	for (de = readdir(dir); de; de = readdir(dir)) {
+		if (!strcmp(de->d_name, ".") ||
+		    !strcmp(de->d_name, ".."))
+			continue;
+
+		fd_no = atoi(de->d_name);
+
+		if (fd_no == fd_dir)
+			continue;
+
+		/* We don't expect many numbers here */
+		for (i = 0; i < nr_skip_fds; i++) {
+			if (fd_no == skip_fds[i]) {
+				fd_no = -1;
+				break;
+			}
+		}
+
+		if (fd_no == -1)
+			continue;
+
+		if (close(fd_no)) {
+			int saved_errno = errno;
+			say_syserror("popen: fdin: Can't close %d", fd_no);
+			closedir(dir);
+			errno = saved_errno;
+			return -1;
+		}
+	}
+
+	if (closedir(dir)) {
+		say_syserror("popen: fdin: Can't close %s", path);
+		return -1;
+	}
+#else
+	/* FIXME: What about FreeBSD/MachO? */
+	(void)skip_fds;
+	(void)nr_skip_fds;
+#endif
+	return 0;
+}
+
+/**
+ * popen_create - Create new popen handle
+ * @command:	a command to run inside child process
+ * @flags:	child pipe ends specification
+ *
+ * This function creates a new child process and passes it
+ * pipe ends to communicate with child's stdin/stdout/stderr
+ * depending on @flags. Where @flags could be the bitwise or
+ * for the following values:
+ *
+ *  POPEN_FLAG_FD_STDIN		- to write to stdin
+ *  POPEN_FLAG_FD_STDOUT	- to read from stdout
+ *  POPEN_FLAG_FD_STDERR	- to read from stderr
+ *
+ * When need to pass /dev/null descriptor into a child
+ * the following values can be used:
+ *
+ *  POPEN_FLAG_FD_STDIN_DEVNULL
+ *  POPEN_FLAG_FD_STDOUT_DEVNULL
+ *  POPEN_FLAG_FD_STDERR_DEVNULL
+ *
+ * These flags do have no effect if appropriate POPEN_FLAG_FD_STDx
+ * flags are set.
+ *
+ * When need to completely close the descriptors the
+ * following values can be used:
+ *
+ *  POPEN_FLAG_FD_STDIN_CLOSE
+ *  POPEN_FLAG_FD_STDOUT_CLOSE
+ *  POPEN_FLAG_FD_STDERR_CLOSE
+ *
+ * These flags do have no effect if appropriate POPEN_FLAG_FD_STDx
+ * flags are set.
+ *
+ * If none of POPEN_FLAG_FD_STDx flags are specified the child
+ * process will run with all files inherited from a parent.
+ *
+ * By default the @command is executed via "sh -c". To execute
+ * @command directly use the POPEN_FLAG_NOSHELL flag.
+ *
+ * Returns pointer to new popen handle on success,
+ * otherwise NULL returned.
+ */
+struct popen_handle *
+popen_create(const char *command, unsigned int flags)
+{
+	struct popen_handle *handle = NULL;
+
+	char * const argv_native[] = {
+		(char *)command, NULL,
+	};
+        char * const argv_sh[] = {
+		(char *)"sh", (char *)"-c",
+		(char *)command, NULL,
+	};
+	/*
+	 * FIXME: Need to pass env in arguments?
+	 * Better discuss with a team.
+	 */
+	char * const envp[] = { };
+
+	int pfd[POPEN_FLAG_FD_STDEND_BIT][2] = {
+		{-1, -1}, {-1, -1}, {-1, -1},
+	};
+
+	int saved_errno;
+	int ret = -1;
+	pid_t pid;
+	size_t i;
+
+	static const struct {
+		unsigned int	mask;
+		unsigned int	mask_devnull;
+		unsigned int	mask_close;
+		int		fileno;
+		int		*dev_null_fd;
+		int		parent_idx;
+		int		child_idx;
+	} pfd_map[POPEN_FLAG_FD_STDEND_BIT] = {
+		{
+			.mask		= POPEN_FLAG_FD_STDIN,
+			.mask_devnull	= POPEN_FLAG_FD_STDIN_DEVNULL,
+			.mask_close	= POPEN_FLAG_FD_STDIN_CLOSE,
+			.fileno		= STDIN_FILENO,
+			.dev_null_fd	= &dev_null_fd_ro,
+			.parent_idx	= 1,
+			.child_idx	= 0,
+		}, {
+			.mask		= POPEN_FLAG_FD_STDOUT,
+			.mask_devnull	= POPEN_FLAG_FD_STDOUT_DEVNULL,
+			.mask_close	= POPEN_FLAG_FD_STDOUT_CLOSE,
+			.fileno		= STDOUT_FILENO,
+			.dev_null_fd	= &dev_null_fd_wr,
+			.parent_idx	= 0,
+			.child_idx	= 1,
+		}, {
+			.mask		= POPEN_FLAG_FD_STDERR,
+			.mask_devnull	= POPEN_FLAG_FD_STDERR_DEVNULL,
+			.mask_close	= POPEN_FLAG_FD_STDERR_CLOSE,
+			.fileno		= STDERR_FILENO,
+			.dev_null_fd	= &dev_null_fd_wr,
+			.parent_idx	= 0,
+			.child_idx	= 1,
+		},
+	};
+	/*
+	 * At max we could be skipping each pipe end
+	 * plus dev/null variants.
+	 */
+	int skip_fds[POPEN_FLAG_FD_STDEND_BIT * 2 + 2];
+	size_t nr_skip_fds = 0;
+
+	sigset_t oldmask;
+
+	say_debug("popen: command \"%s\" flags %#x", command, flags);
+
+	if (!command) {
+		errno = EINVAL;
+		say_syserror("popen: No command provided");
+		return NULL;
+	}
+
+	/*
+	 * If sometime earlier we've been unable
+	 * to unblock signals don't allow to create
+	 * new pipes, the system is unstable.
+	 */
+	if (unlikely(popen_blockmask_broken)) {
+		errno = EINVAL;
+		say_error("popen: Service unavailable");
+		return NULL;
+	}
+
+	static_assert(STDIN_FILENO == 0 &&
+		      STDOUT_FILENO == 1 &&
+		      STDERR_FILENO == 2,
+		      "stdin/out/err are not posix compatible");
+
+	static_assert(lengthof(pfd) == lengthof(pfd_map),
+		      "Pipes number does not map to fd bits");
+
+	static_assert(POPEN_FLAG_FD_STDIN_BIT == STDIN_FILENO &&
+		      POPEN_FLAG_FD_STDOUT_BIT == STDOUT_FILENO &&
+		      POPEN_FLAG_FD_STDERR_BIT == STDERR_FILENO,
+		      "Popen flags do not match stdX");
+
+	handle = handle_alloc(flags);
+	if (!handle)
+		return NULL;
+
+	skip_fds[nr_skip_fds++] = dev_null_fd_ro;
+	skip_fds[nr_skip_fds++] = dev_null_fd_wr;
+	assert(nr_skip_fds <= lengthof(skip_fds));
+
+	for (i = 0; i < lengthof(pfd_map); i++) {
+		if (flags & pfd_map[i].mask) {
+			if (create_pipe(pfd[i]))
+				goto out_err;
+
+			skip_fds[nr_skip_fds++] = pfd[i][0];
+			skip_fds[nr_skip_fds++] = pfd[i][1];
+			assert(nr_skip_fds <= lengthof(skip_fds));
+
+			say_debug("popen: created pipe %d [%d:%d]",
+				  i, pfd[i][0], pfd[i][1]);
+		} else if (!(flags & pfd_map[i].mask_devnull) &&
+			   !(flags & pfd_map[i].mask_close)) {
+			skip_fds[nr_skip_fds++] = pfd_map[i].fileno;
+
+			say_debug("popen: inherit fd %d",
+				  pfd_map[i].fileno);
+		}
+	}
+
+	/*
+	 * Need to block signals so we won't hit
+	 * a race where child process exit early
+	 * and this pid will get reused by someone
+	 * else (remember the libev wait() by self).
+	 */
+	if (popen_sigchld_block(&oldmask)) {
+		say_syserror("popen: Unable to block SIGCHLD");
+		goto out_err;
+	}
+
+	/*
+	 * We have to use vfork here because libev has own
+	 * at_fork helpers with mutex, so we will have double
+	 * lock here and stuck forever otherwise.
+	 *
+	 * The good news that this affect coio only the
+	 * other tarantoll threads are not waiting for
+	 * vfork to complete.
+	 */
+	handle->pid = vfork();
+	if (handle->pid < 0) {
+		goto out_err_unblock;
+	} else if (handle->pid == 0) {
+		/*
+		 * The documentation for libev says that
+		 * each new fork should call ev_loop_fork(EV_DEFAULT)
+		 * on every new child process, but we're going
+		 * to execute bew binary anyway thus everything
+		 * related to OS resources will be eliminated except
+		 * file descriptors we use for piping. Thus don't
+		 * do anything special.
+		 */
+
+		/*
+		 * We have to be a session leader otherwise
+		 * won't be able to kill a group of children.
+		 */
+		ret = setsid();
+		if (ret < 0)
+			_exit(errno);
+
+		ret = close_inherited_fds(skip_fds, nr_skip_fds);
+		if (ret)
+			_exit(errno);
+
+		for (i = 0; !ret && i < lengthof(pfd_map); i++) {
+			int fileno = pfd_map[i].fileno;
+			if (flags & pfd_map[i].mask) {
+				int child_idx = pfd_map[i].child_idx;
+
+				/* put child peer end at known place */
+				if (dup2(pfd[i][child_idx], fileno) < 0) {
+					ret = errno;
+					continue;
+				}
+
+				/* parent's pipe no longer needed */
+				if (close(pfd[i][0])) {
+					ret = errno;
+					continue;
+				} else if (close(pfd[i][1])) {
+					ret = errno;
+					continue;
+				}
+			} else {
+				/* Use /dev/null if requested */
+				if (flags & pfd_map[i].mask_devnull) {
+					if (dup2(*pfd_map[i].dev_null_fd, fileno) < 0) {
+						ret = errno;
+						continue;
+					}
+				}
+
+				/* Or close the destination completely */
+				if (flags & pfd_map[i].mask_close) {
+					if (close(fileno) && errno != EBADF) {
+						ret = errno;
+						continue;
+					}
+				}
+
+				/* Otherwise inherit from a parent */
+			}
+		}
+
+		if (close(dev_null_fd_ro))
+			ret = errno;
+		else if (close(dev_null_fd_wr))
+			ret = errno;
+
+		if (!ret) {
+			if (flags & POPEN_FLAG_SHELL)
+				ret = execve(_PATH_BSHELL, argv_sh, envp);
+			else
+				ret = execve(command, argv_native, envp);
+		}
+		_exit(ret);
+		unreachable();
+	}
+
+	for (i = 0; i < lengthof(pfd_map); i++) {
+		if (flags & pfd_map[i].mask) {
+			int parent_idx = pfd_map[i].parent_idx;
+			int child_idx = pfd_map[i].child_idx;
+
+			handle->fds[i] = pfd[i][parent_idx];
+			say_debug("popen: keep pipe %d [%d]",
+				  i, handle->fds[i]);
+
+			if (close(pfd[i][child_idx]))
+				goto out_err_unblock;
+
+			pfd[i][child_idx] = -1;
+		}
+	}
+
+	pid = popen_wstatus_blocked(handle, NULL, true);
+	if (pid == -1) {
+		say_debug("popen: Child %d exited early",
+			  handle->pid);
+		handle->pid = -1;
+	} else if (pid == 1) {
+		bool exited = WIFEXITED(handle->wstatus);
+		bool signaled = WIFSIGNALED(handle->wstatus);
+
+		if (exited || signaled) {
+			say_debug("popen: Child %d %s with %d",
+				  pid, exited ? "exited" : "signaled",
+				  exited ? WEXITSTATUS(handle->wstatus) :
+				  WTERMSIG(handle->wstatus));
+			handle->pid = -1;
+		}
+	}
+
+	/*
+	 * Link it into global list for force
+	 * cleanup on exit.
+	 */
+	rlist_add(&popen_head, &handle->list);
+
+	if (handle->pid != -1) {
+		/*
+		 * To watch when a child get exited.
+		 */
+		popen_register(handle);
+
+		say_debug("popen: ev_child_start %d", handle->pid);
+		ev_child_init(&handle->ev_sigchld, ev_sigchld_cb, pid, 0);
+		ev_child_start(EV_DEFAULT_ &handle->ev_sigchld);
+	}
+
+	say_debug("popen: created child %d", handle->pid);
+
+	popen_sigchld_unblock(&oldmask);
+	return handle;
+
+out_err_unblock:
+	saved_errno = errno;
+	popen_sigchld_unblock(&oldmask);
+	errno = saved_errno;
+out_err:
+	saved_errno = errno;
+	popen_destroy(handle);
+	for (i = 0; i < lengthof(pfd); i++) {
+		if (pfd[i][0] != -1)
+			close(pfd[i][0]);
+		if (pfd[i][1] != -1)
+			close(pfd[i][1]);
+	}
+	errno = saved_errno;
+	return NULL;
+}
+
+/**
+ * popen_init - initialize popen subsystem
+ *
+ * Allocates resource needed for popen management.
+ */
+void
+popen_init(void)
+{
+	static const int flags = O_CLOEXEC;
+	static const char dev_null_path[] = "/dev/null";
+
+	say_debug("popen: initialize");
+	popen_pids_map = mh_i32ptr_new();
+
+	dev_null_fd_ro = open(dev_null_path, O_RDONLY | flags);
+	if (dev_null_fd_ro < 0)
+		goto out_err;
+	dev_null_fd_wr = open(dev_null_path, O_WRONLY | flags);
+	if (dev_null_fd_wr < 0)
+		goto out_err;
+
+	/*
+	 * FIXME: We should allocate them somewhere
+	 * after STDERR_FILENO so the child would be
+	 * able to find these fd numbers not occupied.
+	 * Other option is to use unix scm and send
+	 * them to the child on demand.
+	 *
+	 * For now left as is since we don't close
+	 * our main stdX descriptors and they are
+	 * inherited when we call first vfork.
+	 */
+	if (dev_null_fd_ro <= STDERR_FILENO ||
+	    dev_null_fd_wr <= STDERR_FILENO) {
+		say_error("popen: /dev/null %d %d numbers are too low",
+			  dev_null_fd_ro, dev_null_fd_wr);
+	}
+
+	sigemptyset(&popen_blockmask);
+	sigaddset(&popen_blockmask, SIGCHLD);
+	return;
+
+out_err:
+	say_syserror("popen: Can't open %s", dev_null_path);
+	if (dev_null_fd_ro >= 0)
+		close(dev_null_fd_ro);
+	if (dev_null_fd_wr >= 0)
+		close(dev_null_fd_wr);
+	mh_i32ptr_delete(popen_pids_map);
+	exit(EXIT_FAILURE);
+}
+
+/**
+ * popen_fini - finalize popen subsystem
+ *
+ * Kills all running children and frees resources.
+ */
+void
+popen_fini(void)
+{
+	struct popen_handle *handle, *tmp;
+	sigset_t oldmask;
+
+	say_debug("popen: finalize");
+
+	close(dev_null_fd_ro);
+	close(dev_null_fd_wr);
+	dev_null_fd_ro = -1;
+	dev_null_fd_wr = -1;
+
+	if (popen_sigchld_block(&oldmask))
+		return;
+
+	rlist_foreach_entry_safe(handle, &popen_head, list, tmp) {
+		/*
+		 * If children are still running we should move
+		 * them out of the pool and kill them all then.
+		 * Note though that we don't do an explicit wait
+		 * here, OS will reap them anyway, just release
+		 * the memory occupied for popen handles.
+		 */
+		if (popen_may_pidop(handle))
+			popen_unregister(handle);
+		popen_destroy_blocked(handle);
+	}
+
+	popen_sigchld_unblock(&oldmask);
+
+	if (popen_pids_map) {
+		mh_i32ptr_delete(popen_pids_map);
+		popen_pids_map = NULL;
+	}
+}
diff --git a/src/lib/core/popen.h b/src/lib/core/popen.h
new file mode 100644
index 000000000..471607662
--- /dev/null
+++ b/src/lib/core/popen.h
@@ -0,0 +1,135 @@
+#ifndef TARANTOOL_LIB_CORE_POPEN_H_INCLUDED
+#define TARANTOOL_LIB_CORE_POPEN_H_INCLUDED
+
+#if defined(__cplusplus)
+extern "C" {
+#endif
+
+#include <signal.h>
+#include <sys/types.h>
+
+#include <small/rlist.h>
+
+#include "third_party/tarantool_ev.h"
+
+enum popen_flag_bits {
+	POPEN_FLAG_NONE			= (0 << 0),
+
+	/*
+	 * Which fd we should handle.
+	 */
+	POPEN_FLAG_FD_STDIN_BIT		= 0,
+	POPEN_FLAG_FD_STDIN		= (1 << POPEN_FLAG_FD_STDIN_BIT),
+
+	POPEN_FLAG_FD_STDOUT_BIT	= 1,
+	POPEN_FLAG_FD_STDOUT		= (1 << POPEN_FLAG_FD_STDOUT_BIT),
+
+	POPEN_FLAG_FD_STDERR_BIT	= 2,
+	POPEN_FLAG_FD_STDERR		= (1 << POPEN_FLAG_FD_STDERR_BIT),
+
+	/*
+	 * Number of bits occupied for stdX descriptors.
+	 */
+	POPEN_FLAG_FD_STDEND_BIT	= POPEN_FLAG_FD_STDERR_BIT + 1,
+
+	/*
+	 * Instead of inheriting fds from a parent
+	 * rather use /dev/null.
+	 */
+	POPEN_FLAG_FD_STDIN_DEVNULL_BIT	= 3,
+	POPEN_FLAG_FD_STDIN_DEVNULL	= (1 << POPEN_FLAG_FD_STDIN_DEVNULL_BIT),
+	POPEN_FLAG_FD_STDOUT_DEVNULL_BIT= 4,
+	POPEN_FLAG_FD_STDOUT_DEVNULL	= (1 << POPEN_FLAG_FD_STDOUT_DEVNULL_BIT),
+	POPEN_FLAG_FD_STDERR_DEVNULL_BIT= 5,
+	POPEN_FLAG_FD_STDERR_DEVNULL	= (1 << POPEN_FLAG_FD_STDERR_DEVNULL_BIT),
+
+	/*
+	 * Instead of inheriting fds from a parent
+	 * close fds completely.
+	 */
+	POPEN_FLAG_FD_STDIN_CLOSE_BIT	= 6,
+	POPEN_FLAG_FD_STDIN_CLOSE	= (1 << POPEN_FLAG_FD_STDIN_CLOSE_BIT),
+	POPEN_FLAG_FD_STDOUT_CLOSE_BIT	= 7,
+	POPEN_FLAG_FD_STDOUT_CLOSE	= (1 << POPEN_FLAG_FD_STDOUT_CLOSE_BIT),
+	POPEN_FLAG_FD_STDERR_CLOSE_BIT	= 8,
+	POPEN_FLAG_FD_STDERR_CLOSE	= (1 << POPEN_FLAG_FD_STDERR_CLOSE_BIT),
+
+	/*
+	 * Call exec directly or via shell.
+	 */
+	POPEN_FLAG_SHELL_BIT		= 9,
+	POPEN_FLAG_SHELL		= (1 << POPEN_FLAG_SHELL_BIT),
+};
+
+/**
+ * struct popen_handle - an instance of popen object
+ *
+ * @pid:	pid of a child process
+ * @wstatus:	exit status of a child process
+ * @ev_sigchld:	needed by the libev to watch children
+ * @flags:	popen_flag_bits
+ * @fds:	std(in|out|err)
+ */
+struct popen_handle {
+	pid_t			pid;
+	int			wstatus;
+	ev_child		ev_sigchld;
+	struct rlist		list;
+	unsigned int		flags;
+	int			fds[POPEN_FLAG_FD_STDEND_BIT];
+};
+
+/**
+ * struct popen_handle - popen object statistics
+ *
+ * @pid:	pid of a child process
+ * @wstatus:	exit status of a child process
+ * @flags:	popen_flag_bits
+ * @fds:	std(in|out|err)
+ *
+ * This is a short version of struct popen_handle which should
+ * be used by external code and which should be changed/extended
+ * with extreme caution since it is used in Lua code. Consider it
+ * as API for external modules.
+ */
+struct popen_stat {
+	pid_t			pid;
+	unsigned int		flags;
+	int			fds[POPEN_FLAG_FD_STDEND_BIT];
+};
+
+extern int
+popen_stat(struct popen_handle *handle, struct popen_stat *st);
+
+extern ssize_t
+popen_write(struct popen_handle *handle, void *buf,
+	    size_t count, unsigned int flags);
+
+extern ssize_t
+popen_read_timeout(struct popen_handle *handle, void *buf,
+		   size_t count, unsigned int flags,
+		   int timeout_msecs);
+
+extern int
+popen_wstatus(struct popen_handle *handle, int *wstatus);
+
+extern int
+popen_kill(struct popen_handle *handle);
+
+extern int
+popen_destroy(struct popen_handle *handle);
+
+extern struct popen_handle *
+popen_create(const char *command, unsigned int flags);
+
+extern void
+popen_init(void);
+
+extern void
+popen_fini(void);
+
+#if defined(__cplusplus)
+} /* extern "C" */
+#endif
+
+#endif /* TARANTOOL_LIB_CORE_POPEN_H_INCLUDED */
diff --git a/src/main.cc b/src/main.cc
index 0ff2213b6..a9999b47d 100644
--- a/src/main.cc
+++ b/src/main.cc
@@ -77,6 +77,7 @@
 #include "box/session.h"
 #include "systemd.h"
 #include "crypto/crypto.h"
+#include "core/popen.h"
 
 static pid_t master_pid = getpid();
 static struct pidfh *pid_file_handle;
@@ -614,6 +615,8 @@ tarantool_free(void)
 
 	title_free(main_argc, main_argv);
 
+	popen_fini();
+
 	/* unlink pidfile. */
 	if (pid_file_handle != NULL && pidfile_remove(pid_file_handle) == -1)
 		say_syserror("failed to remove pid file '%s'", pid_file);
@@ -801,6 +804,7 @@ main(int argc, char **argv)
 	exception_init();
 
 	fiber_init(fiber_cxx_invoke);
+	popen_init();
 	coio_init();
 	coio_enable();
 	signal_init();
-- 
2.20.1

[Tarantool-patches] [PATCH 2/5] lua/fio: Add lbox_fio_push_error as a separate helper

[Cyrill Gorcunov]

Since lbox_fio_pushbool always push boolean first
it is inpossible to use it inside other routines.
Thus make a separate helper lbox_fio_push_error
which will be used in popen code later.

Part-of #4031

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/lua/fio.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/lua/fio.c b/src/lua/fio.c
index 7437cc0c6..33ccd5d71 100644
--- a/src/lua/fio.c
+++ b/src/lua/fio.c
@@ -52,6 +52,16 @@
 	luaT_push_nil_and_error(L);				\
 })
 
+static inline int
+lbox_fio_push_error(struct lua_State *L)
+{
+	diag_set(SystemError, "fio: %s", strerror(errno));
+	struct error *e = diag_last_error(diag_get());
+	assert(e != NULL);
+	luaT_pusherror(L, e);
+	return 1;
+}
+
 static int
 lbox_fio_open(struct lua_State *L)
 {
@@ -118,10 +128,7 @@ lbox_fio_pushbool(struct lua_State *L, bool res)
 {
 	lua_pushboolean(L, res);
 	if (!res) {
-		diag_set(SystemError, "fio: %s", strerror(errno));
-		struct error *e = diag_last_error(diag_get());
-		assert(e != NULL);
-		luaT_pusherror(L, e);
+		lbox_fio_push_error(L);
 		return 2;
 	}
 	return 1;
-- 
2.20.1

[Tarantool-patches] [PATCH 3/5] popen/fio: Merge popen engine into fio internal module

[Cyrill Gorcunov]

In the patch we wrap popen calls with lua C interface
which will allow us to use popen inside plain lua code.
They are wrapped via coio_custom calls thus each routine
takes into account current libev priority state.

Worth to note again that the creation of a new popen object
may cause coio thread to stall until exec/exit called. Should
not be a huge problem, after all exec itself is extremelly
heavy syscall.

Part-of #4031

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/lib/core/coio_file.c | 155 ++++++++++++++++++++++++++
 src/lib/core/coio_file.h |  10 ++
 src/lua/fio.c            | 235 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 400 insertions(+)

diff --git a/src/lib/core/coio_file.c b/src/lib/core/coio_file.c
index 62388344e..39e4ed193 100644
--- a/src/lib/core/coio_file.c
+++ b/src/lib/core/coio_file.c
@@ -30,6 +30,7 @@
  */
 #include "coio_file.h"
 #include "coio_task.h"
+#include "popen.h"
 #include "fiber.h"
 #include "say.h"
 #include "fio.h"
@@ -103,6 +104,16 @@ struct coio_file_task {
 			const char *source;
 			const char *dest;
 		} copyfile;
+
+		struct {
+			struct popen_handle *handle;
+			const char *command;
+			unsigned int flags;
+			int timeout_msecs;
+			size_t count;
+			int wstatus;
+			void *buf;
+		} popen;
 	};
 };
 
@@ -639,3 +650,147 @@ coio_utime(const char *pathname, double atime, double mtime)
 	eio_req *req = eio_utime(pathname, atime, mtime, 0, coio_complete, &eio);
 	return coio_wait_done(req, &eio);
 }
+
+static void
+do_coio_popen_create(eio_req *req)
+{
+	struct coio_file_task *eio = req->data;
+
+	eio->popen.handle = popen_create(eio->popen.command,
+					 eio->popen.flags);
+	req->result = eio->popen.handle ? 0 : -1;
+}
+
+void *
+coio_popen_create(const char *command, unsigned int flags)
+{
+	INIT_COEIO_FILE(eio);
+	eio.popen.command = command;
+	eio.popen.flags = flags;
+
+	eio_req *req = eio_custom(do_coio_popen_create,
+				  EIO_PRI_DEFAULT,
+				  coio_complete, &eio);
+	coio_wait_done(req, &eio);
+
+	return eio.popen.handle;
+}
+
+static void
+coio_do_popen_destroy(eio_req *req)
+{
+        struct coio_file_task *eio = req->data;
+	req->result = popen_destroy(eio->popen.handle);
+}
+
+int
+coio_popen_destroy(void *handle)
+{
+	INIT_COEIO_FILE(eio);
+	eio.popen.handle = handle;
+
+	eio_req *req = eio_custom(coio_do_popen_destroy,
+				  EIO_PRI_DEFAULT,
+				  coio_complete, &eio);
+	return coio_wait_done(req, &eio);
+}
+
+static void
+coio_do_popen_kill(eio_req *req)
+{
+        struct coio_file_task *eio = req->data;
+	req->result = popen_kill(eio->popen.handle);
+}
+
+int
+coio_popen_kill(void *handle)
+{
+	INIT_COEIO_FILE(eio);
+	eio.popen.handle = handle;
+
+	eio_req *req = eio_custom(coio_do_popen_kill,
+				  EIO_PRI_DEFAULT,
+				  coio_complete, &eio);
+	return coio_wait_done(req, &eio);
+}
+
+static void
+coio_do_popen_status(eio_req *req)
+{
+        struct coio_file_task *eio = req->data;
+	req->result = popen_wstatus(eio->popen.handle,
+				    &eio->popen.wstatus);
+}
+
+int
+coio_popen_status(void *handle, int *reason, int *exit_code)
+{
+	INIT_COEIO_FILE(eio);
+	eio.popen.handle = handle;
+
+	eio_req *req = eio_custom(coio_do_popen_status,
+				  EIO_PRI_DEFAULT,
+				  coio_complete, &eio);
+	int ret = coio_wait_done(req, &eio);
+	if (ret > 0) {
+		*reason = WIFEXITED(eio.popen.wstatus) ?  1 : 2;
+		*exit_code = WIFEXITED(eio.popen.wstatus) ?
+			WEXITSTATUS(eio.popen.wstatus) :
+			WTERMSIG(eio.popen.wstatus);
+	}
+	return ret;
+}
+
+static void
+coio_do_popen_read(eio_req *req)
+{
+	struct coio_file_task *eio = req->data;
+	req->result = popen_read_timeout(eio->popen.handle,
+					 eio->popen.buf,
+					 eio->popen.count,
+					 eio->popen.flags,
+					 eio->popen.timeout_msecs);
+}
+
+ssize_t
+coio_popen_read(void *handle, void *buf, size_t count,
+		unsigned int flags, int timeout_msecs)
+{
+	INIT_COEIO_FILE(eio);
+	eio.popen.timeout_msecs = timeout_msecs;
+	eio.popen.handle = handle;
+	eio.popen.flags	= flags;
+	eio.popen.count	= count;
+	eio.popen.buf = buf;
+
+	eio_req *req = eio_custom(coio_do_popen_read,
+				  EIO_PRI_DEFAULT,
+				  coio_complete, &eio);
+	return coio_wait_done(req, &eio);
+}
+
+static void
+coio_do_popen_write(eio_req *req)
+{
+	struct coio_file_task *eio = req->data;
+	req->result = popen_write(eio->popen.handle,
+				  eio->popen.buf,
+				  eio->popen.count,
+				  eio->popen.flags);
+}
+
+ssize_t
+coio_popen_write(void *handle, void *buf, size_t count,
+		 unsigned int flags)
+{
+	INIT_COEIO_FILE(eio);
+	eio.popen.handle = handle;
+	eio.popen.flags	= flags;
+	eio.popen.count	= count;
+	eio.popen.buf = buf;
+
+	eio_req *req = eio_custom(coio_do_popen_write,
+				  EIO_PRI_DEFAULT,
+				  coio_complete, &eio);
+	return coio_wait_done(req, &eio);
+}
diff --git a/src/lib/core/coio_file.h b/src/lib/core/coio_file.h
index ac7b1aacf..fb385737d 100644
--- a/src/lib/core/coio_file.h
+++ b/src/lib/core/coio_file.h
@@ -85,6 +85,16 @@ int	coio_tempdir(char *path, size_t path_len);
 int	coio_readdir(const char *path, char **buf);
 int	coio_copyfile(const char *source, const char *dest);
 int	coio_utime(const char *pathname, double atime, double mtime);
+
+void	*coio_popen_create(const char *command, unsigned int flags);
+int	coio_popen_destroy(void *handle);
+int	coio_popen_kill(void *handle);
+int	coio_popen_status(void *handle, int *reason, int *exit_code);
+ssize_t	coio_popen_read(void *handle, void *buf, size_t count,
+			unsigned int flags, int timeout_msecs);
+ssize_t coio_popen_write(void *handle, void *buf, size_t count,
+			 unsigned int flags);
+
 #if defined(__cplusplus)
 } /* extern "C" */
 #endif /* defined(__cplusplus) */
diff --git a/src/lua/fio.c b/src/lua/fio.c
index 33ccd5d71..3cf52f2d5 100644
--- a/src/lua/fio.c
+++ b/src/lua/fio.c
@@ -39,6 +39,7 @@
 #include <time.h>
 #include <errno.h>
 #include "coio_task.h"
+#include "popen.h"
 
 #include <lua.h>
 #include <lauxlib.h>
@@ -683,6 +684,233 @@ lbox_fio_utime(struct lua_State *L)
 	return lbox_fio_pushbool(L, coio_utime(pathname, atime, mtime) == 0);
 }
 
+/**
+ * lbox_fio_popen_create - creates a new popen handle and runs a command inside
+ * @command:	a command to run
+ * @flags:	child pipe end specification
+ *
+ * Returns pair @handle = data, @err = nil on success,
+ * @handle = nil, err ~= nil on error.
+ */
+static int
+lbox_fio_popen_create(struct lua_State *L)
+{
+	struct popen_handle *handle;
+	unsigned int flags;
+	const char *command;
+
+	if (lua_gettop(L) < 1) {
+usage:
+		luaL_error(L, "Usage: fio.popen(command [, rw, {opts}])");
+	}
+
+	command = lua_tostring(L, 1);
+	if (!command)
+		goto usage;
+	flags = lua_tonumber(L, 2);
+
+	handle = coio_popen_create(command, flags);
+	if (!handle)
+		return lbox_fio_pushsyserror(L);
+
+	lua_pushlightuserdata(L, handle);
+	return 1;
+}
+
+/**
+ * lbox_fio_popen_destroy - close a popen handle
+ * @handle:	a handle to close
+ *
+ * If there is a running child it get killed first.
+ *
+ * Returns true if a handle is closeed, false otherwise.
+ */
+static int
+lbox_fio_popen_destroy(struct lua_State *L)
+{
+	void *handle = lua_touserdata(L, 1);
+	return lbox_fio_pushbool(L, coio_popen_destroy(handle) == 0);
+}
+
+/**
+ * lbox_fio_popen_kill - kill popen's child process
+ * @handle:	a handle carries child process to kill
+ *
+ * Returns true if process is killed and false
+ * otherwise. Note the process is simply signaled
+ * and it doesn't mean it is killed immediately,
+ * Poll lbox_fio_pstatus if need to find out when
+ * exactly the child is reaped out.
+ */
+static int
+lbox_fio_popen_kill(struct lua_State *L)
+{
+	struct popen_handle *p = lua_touserdata(L, 1);
+	return lbox_fio_pushbool(L, coio_popen_kill(p) == 0);
+}
+
+/**
+ * lbox_fio_popen_status - fetch popen child process status
+ * @handle:	a handle to fetch status from
+ *
+ * Returns @err = nil, @reason = [1|2], @exit_code = 'number'
+ * on success, where reason = 1 means process has been calling
+ * exit(@exit_code); and reason = 2 when process has been killed
+ * by @exit_code signal (either explicitly via lbox_fio_pkill
+ * or implicitly by a third side, say node admin via kill command
+ * in a shell).
+ */
+static int
+lbox_fio_popen_status(struct lua_State *L)
+{
+	struct popen_handle *p = lua_touserdata(L, 1);
+	int reason = 0, exit_code = 0, ret;
+
+	ret = coio_popen_status(p, &reason, &exit_code);
+	if (ret < 0) {
+		return lbox_fio_push_error(L);
+	} else if (ret == 0) {
+		lua_pushnil(L);
+		return 1;
+	}
+
+	lua_pushnil(L);
+	lua_pushinteger(L, reason);
+	lua_pushinteger(L, exit_code);
+	return 3;
+}
+
+/**
+ * lbox_fio_popen_read - read data from a child peer
+ * @handle:		a handle of a child process
+ * @buf:		destination buffer
+ * @count:		number of bytes to read
+ * @timeout_msecs:	read timeout, in microseconds, -1 to not use
+ * @flags:		which peer to read (stdout,stderr)
+ *
+ * Returns @size = 'read bytes', @err = nil on success,
+ * @size = nil, @err = nil if timeout expired, and
+ * @size = nil, @err ~= nil on error.
+ */
+static int
+lbox_fio_popen_read(struct lua_State *L)
+{
+	struct popen_handle *handle = lua_touserdata(L, 1);
+	uint32_t ctypeid;
+	void *buf =  *(char **)luaL_checkcdata(L, 2, &ctypeid);
+	size_t count = lua_tonumber(L, 3);
+	int timeout_msecs = lua_tonumber(L, 4);
+	unsigned int flags = lua_tonumber(L, 5);
+	ssize_t ret;
+
+	ret = coio_popen_read(handle, buf, count, flags, timeout_msecs);
+	if (ret < 0) {
+		if (ret == -EAGAIN) {
+			lua_pushnil(L);
+			lua_pushnil(L);
+			return 2;
+		} else
+			return lbox_fio_pushsyserror(L);
+	}
+
+	lua_pushinteger(L, ret);
+	return 1;
+}
+
+/**
+ * lbox_fio_popen_write - write data to a child peer
+ * @handle:	a handle of a child process
+ * @buf:	source buffer
+ * @count:	number of bytes to write
+ * @flags:	which peer to write (stdin)
+ *
+ * Returns @size = 'bytes wrote', @err = nil on succes,
+ * @size = nil, @err ~= nil on error.
+ */
+static int
+lbox_fio_popen_write(struct lua_State *L)
+{
+	struct popen_handle *handle = lua_touserdata(L, 1);
+	void *buf = (void *)lua_tostring(L, 2);
+	uint32_t ctypeid = 0;
+	if (buf == NULL)
+		buf =  *(char **)luaL_checkcdata(L, 2, &ctypeid);
+	size_t count = lua_tonumber(L, 3);
+	unsigned int flags = lua_tonumber(L, 4);
+	ssize_t ret;
+
+	ret = coio_popen_write(handle, buf, count, flags);
+	if (ret < 0)
+		return lbox_fio_pushsyserror(L);
+
+	lua_pushinteger(L, ret);
+	return 1;
+}
+
+/**
+ * lbox_fio_popen_info - return information about popen handle
+ * @handle:	a handle of a child process
+ *
+ * Returns a @table ~= nil, @err = nil on success,
+ * @table = nil, @err ~= nil on error.
+ */
+static int
+lbox_fio_popen_info(struct lua_State *L)
+{
+	struct popen_handle *handle = lua_touserdata(L, 1);
+	int reason = 0, exit_code = 0, ret;
+	struct popen_stat st = { };
+	char *status;
+
+	if (popen_stat(handle, &st))
+		return lbox_fio_pushsyserror(L);
+
+	ret = coio_popen_status(handle, &reason, &exit_code);
+	if (ret < 0)
+		return lbox_fio_pushsyserror(L);
+
+	if (ret == 0)
+		status = "alive";
+	else if (ret == 1)
+		status = "exited";
+	else if (ret == 2)
+		status = "signaled";
+	else
+		status = "unknown";
+
+	lua_newtable(L);
+
+	lua_pushliteral(L, "pid");
+	lua_pushinteger(L, st.pid);
+	lua_settable(L, -3);
+
+	lua_pushliteral(L, "flags");
+	lua_pushinteger(L, st.flags);
+	lua_settable(L, -3);
+
+	lua_pushliteral(L, "state");
+	lua_pushstring(L, status);
+	lua_settable(L, -3);
+
+	lua_pushliteral(L, "exit_code");
+	lua_pushinteger(L, exit_code);
+	lua_settable(L, -3);
+
+	lua_pushliteral(L, "stdin");
+	lua_pushinteger(L, st.fds[POPEN_FLAG_FD_STDIN_BIT]);
+	lua_settable(L, -3);
+
+	lua_pushliteral(L, "stdout");
+	lua_pushinteger(L, st.fds[POPEN_FLAG_FD_STDOUT_BIT]);
+	lua_settable(L, -3);
+
+	lua_pushliteral(L, "stderr");
+	lua_pushinteger(L, st.fds[POPEN_FLAG_FD_STDERR_BIT]);
+	lua_settable(L, -3);
+
+	return 1;
+}
+
 void
 tarantool_lua_fio_init(struct lua_State *L)
 {
@@ -726,6 +954,13 @@ tarantool_lua_fio_init(struct lua_State *L)
 		{ "fstat",		lbox_fio_fstat			},
 		{ "copyfile",		lbox_fio_copyfile,		},
 		{ "utime",		lbox_fio_utime			},
+		{ "popen_create",	lbox_fio_popen_create		},
+		{ "popen_destroy",	lbox_fio_popen_destroy		},
+		{ "popen_kill",		lbox_fio_popen_kill		},
+		{ "popen_status",	lbox_fio_popen_status		},
+		{ "popen_read",		lbox_fio_popen_read		},
+		{ "popen_write",	lbox_fio_popen_write		},
+		{ "popen_info",		lbox_fio_popen_info		},
 		{ NULL,			NULL				}
 	};
 	luaL_register(L, NULL, internal_methods);
-- 
2.20.1

[Tarantool-patches] [PATCH 4/5] popen/fio: Implement lua interface for a popen object

[Cyrill Gorcunov]

In the patch we implement lua interface for popen management.

It provides the following methods:

 - fio.popen() to create a new popen object which basically
   runs an external process
 - popen:info() to fetch information about a process
 - popen:read() to read stdout or stderr of a child process,
   including passing timeout in microseconds to interrupt
   read procedure if no data present on the other peer
 - popen:write() to write into stdin of a child process
 - popen:kill() to kill the process
 - popen:wait() to wait until the process get exited

Part-of #4031

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/lua/fio.lua | 434 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 434 insertions(+)

diff --git a/src/lua/fio.lua b/src/lua/fio.lua
index cb224f3d0..df436c6f1 100644
--- a/src/lua/fio.lua
+++ b/src/lua/fio.lua
@@ -5,6 +5,7 @@ local ffi = require('ffi')
 local buffer = require('buffer')
 local fiber = require('fiber')
 local errno = require('errno')
+local bit = require('bit')
 
 ffi.cdef[[
     int umask(int mask);
@@ -519,4 +520,437 @@ fio.path.lexists = function(filename)
     return fio.lstat(filename) ~= nil
 end
 
+local popen_methods = { }
+
+--
+-- Make sure the bits are matching POPEN_FLAG_x
+-- flags in popen header. This is api but internal
+-- one, we should not make it visible to users.
+local popen_flag_bits = {
+    STDIN           = 0,
+    STDOUT          = 1,
+    STDERR          = 2,
+
+    STDIN_DEVNULL   = 3,
+    STDOUT_DEVNULL  = 4,
+    STDERR_DEVNULL  = 5,
+
+    STDIN_CLOSE     = 6,
+    STDOUT_CLOSE    = 7,
+    STDERR_CLOSE    = 8,
+
+    SHELL           = 9,
+}
+
+local popen_flags = {
+    NONE            = 0,
+    STDIN           = bit.lshift(1, popen_flag_bits.STDIN),
+    STDOUT          = bit.lshift(1, popen_flag_bits.STDOUT),
+    STDERR          = bit.lshift(1, popen_flag_bits.STDERR),
+
+    STDIN_DEVNULL   = bit.lshift(1, popen_flag_bits.STDIN_DEVNULL),
+    STDOUT_DEVNULL  = bit.lshift(1, popen_flag_bits.STDOUT_DEVNULL),
+    STDERR_DEVNULL  = bit.lshift(1, popen_flag_bits.STDERR_DEVNULL),
+
+    STDIN_CLOSE     = bit.lshift(1, popen_flag_bits.STDIN_CLOSE),
+    STDOUT_CLOSE    = bit.lshift(1, popen_flag_bits.STDOUT_CLOSE),
+    STDERR_CLOSE    = bit.lshift(1, popen_flag_bits.STDERR_CLOSE),
+
+    SHELL           = bit.lshift(1, popen_flag_bits.SHELL),
+}
+
+--
+-- Close a popen object and release all resources.
+-- In case if there is a running child process
+-- it will be killed.
+--
+-- Returns @ret = true if popen handle closed, and
+-- @ret = false, @err ~= nil on error.
+popen_methods.close = function(self)
+    local ret, err = internal.popen_destroy(self.popen_handle)
+    if ret == true and err == nil then
+        self.popen_handle = nil
+    end
+    return ret, err
+end
+
+--
+-- Kill a child process
+--
+-- Returns @ret = true on success,
+-- @ret = false, @err ~= nil on error.
+popen_methods.kill = function(self)
+    return internal.popen_kill(self.popen_handle)
+end
+
+--
+-- Fetch a child process status
+--
+-- Returns @err = nil, @reason = (1 - if exited,
+-- 2 - if killed by a signal) and @exit_code ~= nil,
+-- otherwise @err ~= nil.
+--
+-- FIXME: Should the @reason be a named constant?
+popen_methods.status = function(self)
+    return internal.popen_status(self.popen_handle)
+end
+
+--
+-- Wait until a child process finished its work
+-- and exit then.
+--
+-- Since it goes via coio thread we can't simply
+-- call wait4 (or similar) directly to not block
+-- the whole i/o subsystem. Instead we do an explicit
+-- polling for status change with predefined period
+-- of sleeping.
+--
+-- Returns the same as popen_methods.status.
+popen_methods.wait = function(self)
+    local err, reason, code
+    while true do
+        err, reason, code = internal.popen_status(self.popen_handle)
+        if err or reason then
+            break
+        end
+        fiber.sleep(self.wait_secs)
+    end
+    return err, reason, code
+end
+
+--
+-- A map for popen option keys into tfn ('true|false|nil') values
+-- where bits are just set without additional manipulations.
+--
+-- For example stdin=true means to open stdin end for write,
+-- stdin=false to close the end, finally stdin[=nil] means to
+-- provide /dev/null into a child peer.
+local popen_opts_tfn = {
+    stdin = {
+        popen_flags.STDIN,
+        popen_flags.STDIN_CLOSE,
+        popen_flags.STDIN_DEVNULL,
+    },
+    stdout = {
+        popen_flags.STDOUT,
+        popen_flags.STDOUT_CLOSE,
+        popen_flags.STDOUT_DEVNULL,
+    },
+    stderr = {
+        popen_flags.STDERR,
+        popen_flags.STDERR_CLOSE,
+        popen_flags.STDERR_DEVNULL,
+    },
+}
+
+--
+-- A map for popen option keys into tf ('true|false') values
+-- where bits are set on 'true' and clear on 'false'.
+local popen_opts_tf = {
+    shell = {
+        popen_flags.SHELL,
+    },
+}
+
+--
+-- Parses opts table with keys from popen_opts_tfn.
+-- Values could be true, false or nil.
+local function parse_popen_opts(flags, opts)
+    if opts == nil then
+        return flags
+    end
+    for k,v in pairs(opts) do
+        if popen_opts_tfn[k] == nil then
+            if popen_opts_tf[k] == nil then
+                error(string.format("fio.popen: Unknown key %s", k))
+            end
+            if v == true then
+                flags = bit.bor(flags, popen_opts_tf[k][1])
+            elseif v == false then
+                flags = bit.band(flags, bit.bnot(popen_opts_tf[k][1]))
+            else
+                error(string.format("fio.popen: Unknown value %s", v))
+            end
+        else
+            if v == true then
+                flags = bit.band(flags, bit.bnot(popen_opts_tfn[k][2]))
+                flags = bit.band(flags, bit.bnot(popen_opts_tfn[k][3]))
+                flags = bit.bor(flags, popen_opts_tfn[k][1])
+            elseif v == false then
+                flags = bit.band(flags, bit.bnot(popen_opts_tfn[k][1]))
+                flags = bit.band(flags, bit.bnot(popen_opts_tfn[k][3]))
+                flags = bit.bor(flags, popen_opts_tfn[k][2])
+            elseif v == "devnull" then
+                flags = bit.band(flags, bit.bnot(popen_opts_tfn[k][1]))
+                flags = bit.band(flags, bit.bnot(popen_opts_tfn[k][2]))
+                flags = bit.bor(flags, popen_opts_tfn[k][3])
+            else
+                error(string.format("fio.popen: Unknown value %s", v))
+            end
+        end
+    end
+    return flags
+end
+
+--
+-- popen:read2 - read stream of a child with options
+-- @opts:       options table
+--
+-- The options should have the following keys
+--
+-- @flags:          stdout=true or stderr=true
+-- @buf:            const_char_ptr_t buffer
+-- @size:           size of the buffer
+-- @timeout_msecs:  read timeout in microsecond
+--
+-- Returns @res = bytes, err = nil in case if read processed
+-- without errors, @res = nil, @err = nil if timeout elapsed
+-- and no data appeared on the peer, @res = nil, @err ~= nil
+-- otherwise.
+popen_methods.read2 = function(self, opts)
+    --
+    -- We can reuse parse_popen_opts since only
+    -- a few flags we're interesed in -- stdout/stderr
+    local flags = parse_popen_opts(popen_flags.NONE, opts['flags'])
+    local timeout_msecs = -1
+    if opts['buf'] == nil then
+        error("fio.popen:read2 {'buf'} key is missed")
+    elseif opts['size'] == nil then
+        error("fio.popen:read2 {'size'} key is missed")
+    elseif opts['timeout_msecs'] ~= nil then
+        timeout_msecs = tonumber(opts['timeout_msecs'])
+    end
+
+    return internal.popen_read(self.popen_handle, opts['buf'],
+                               tonumber(opts['size']),
+                               timeout_msecs, flags)
+end
+
+-- popen:write2 - write data to a child with options
+-- @opts:       options table
+--
+-- The options should have the following keys
+--
+-- @flags:      stdin=true
+-- @buf:        const_char_ptr_t buffer
+-- @size:       size of the buffer
+--
+-- Returns @res = bytes, err = nil in case if write processed
+-- without errors, or @res = nil, @err ~= nil otherwise.
+popen_methods.write2 = function(self, opts)
+    local flags = parse_popen_opts(popen_flags.NONE, opts['flags'])
+    if opts['buf'] == nil then
+        error("fio.popen:write2 {'buf'} key is missed")
+    elseif opts['size'] == nil then
+        error("fio.popen:write2 {'size'} key is missed")
+    end
+    return internal.popen_write(self.popen_handle, opts['buf'],
+                                tonumber(opts['size']), flags)
+end
+
+--
+-- Write data to stdin of a child process
+-- @buf:        a buffer to write
+-- @size:       size of the buffer
+--
+-- Both parameters are optional and some
+-- string could be passed instead
+--
+-- write(str)
+-- write(buf, len)
+--
+-- Returns @res = bytes, @err = nil in case of success,
+-- @res = , @err ~= nil in case of error.
+popen_methods.write = function(self, buf, size)
+    if not ffi.istype(const_char_ptr_t, buf) then
+        buf = tostring(buf)
+        size = #buf
+    end
+    return self:write2({ buf = buf, size = size,
+                       flags = { stdin = true }})
+end
+
+--
+-- popen:read - read the output stream in blocked mode
+-- @buf:            a buffer to read to, optional
+-- @size:           size of the buffer, optional
+-- @opts:           options table, optional
+--
+-- The options may have the following keys:
+--
+-- @flags:          stdout=true or stderr=true,
+--                  to specify which stream to read;
+--                  by default stdout is used
+-- @timeout_msecs:  read timeout in microcesonds, when
+--                  specified the read will be interrupted
+--                  after specified time elapced, to make
+--                  nonblocking read; de default the read
+--                  is blocking operation
+--
+-- The following variants are accepted as well
+--
+--  read()          -> str
+--  read(buf)       -> len
+--  read(size)      -> str
+--  read(buf, size) -> len
+--
+-- FIXME: Should we merged it with plain fio.write()?
+popen_methods.read = function(self, buf, size, opts)
+    local tmpbuf
+
+    -- read() or read(buf)
+    if (not ffi.istype(const_char_ptr_t, buf) and buf == nil) or
+        (ffi.istype(const_char_ptr_t, buf) and size == nil) then
+        -- read @self.read_buf_size bytes at once if nothing specified
+        -- FIXME: should it be configurable on popen creation?
+        size = self.read_buf_size
+    end
+
+    -- read(size)
+    if not ffi.istype(const_char_ptr_t, buf) then
+        size = buf or size
+        tmpbuf = buffer.ibuf()
+        buf = tmpbuf:reserve(size)
+    end
+
+    if opts == nil then
+        opts = {
+            timeout_msecs = -1,
+            flags = { stdout = true },
+        }
+    end
+
+    local res, err = self:read2({ buf = buf, size = size,
+                                timeout_msecs = opts['timeout_msecs'],
+                                flags = opts['flags'] })
+
+    if res == nil then
+        if tmpbuf ~= nil then
+            tmpbuf:recycle()
+        end
+        return nil, err
+    end
+
+    if tmpbuf ~= nil then
+        tmpbuf:alloc(res)
+        res = ffi.string(tmpbuf.rpos, tmpbuf:size())
+        tmpbuf:recycle()
+    end
+
+    return res
+end
+
+--
+-- popen:info -- obtain information about a popen object
+--
+-- Returns @info table, err == nil on success,
+-- @info = nil, @err ~= nil otherwise.
+--
+-- The @info table contains the following keys:
+--
+-- @pid:        pid of a child process
+-- @flags:      flags associated (popen_flags bitmap)
+-- @stdout:     parent peer for stdout, -1 if closed
+-- @stderr:     parent peer for stderr, -1 if closed
+-- @stdin:      parent peer for stdin, -1 if closed
+-- @state:      alive | exited | signaled | unknown
+-- @exit_code:  exit code of a child process if been
+--              exited or killed by a signal
+--
+-- The child should never be in "unknown" state, reserved
+-- for unexpected errors.
+--
+popen_methods.info = function(self)
+    return internal.popen_info(self.popen_handle)
+end
+
+local popen_mt = { __index = popen_methods }
+
+--
+-- fio.popen - create a new child process and execute a command inside
+-- @command:    a command to run
+-- @mode:       r - to grab child's stdout for read,
+--              w - to obtain stdin for write
+-- @...:        optional parameters table:
+--
+--  stdin=true      alias for @mode=w
+--  stdin=false     to close STDIN_FILENO inside a child process [*]
+--  stdin="devnull" the child will get /dev/null as STDIN_FILENO
+--
+--  stdout=true     alias for @mode=r
+--  stdout=false    to close STDOUT_FILENO inside a child process [*]
+--  stdin="devnull" the child will get /dev/null as STDOUT_FILENO
+--
+--  stderr=true     to read STDERR_FILENO from a child process
+--  stderr=false    to close STDERR_FILENO inside a child process [*]
+--  stdin="devnull" the child will get /dev/null as STDERR_FILENO
+--
+--  shell=true      runs a child process via "sh -c" [*]
+--  shell=false     invokes a child process executable directly
+--
+-- [*] are default values
+--
+-- Examples:
+--
+--  popen = require('fio').popen("date", "r")
+--      runs date as "sh -c date" to read the output,
+--      closing all file descriptors except stdin inside
+--      a child process
+--
+--  popen = require('fio').popen("/usr/bin/date", "r", {shell=false})
+--      invokes date executable without shell to read the output
+--
+fio.popen = function(command, mode, ...)
+    local flags = popen_flags.NONE
+
+    if type(command) ~= 'string' then
+        error("Usage: fio.popen(command[, rw, {opts}])")
+    end
+
+    -- default flags: close everything and use shell
+    flags = bit.bor(flags, popen_flags.STDIN_CLOSE)
+    flags = bit.bor(flags, popen_flags.STDOUT_CLOSE)
+    flags = bit.bor(flags, popen_flags.STDERR_CLOSE)
+    flags = bit.bor(flags, popen_flags.SHELL)
+
+    for i = 1, #mode do
+        local c = mode:sub(i, i)
+        if c == 'r' then
+            flags = bit.band(flags, bit.bnot(popen_flags.STDOUT_CLOSE))
+            flags = bit.bor(flags, popen_flags.STDOUT)
+        elseif c == 'w' then
+            flags = bit.band(flags, bit.bnot(popen_flags.STDIN_CLOSE))
+            flags = bit.bor(flags, popen_flags.STDIN)
+        else
+            error(string.format("fio.popen: Unknown mode %s", c))
+        end
+    end
+
+    --
+    -- Options table can override the mode option
+    if ... ~= nil then
+        flags = parse_popen_opts(flags, ...)
+    end
+
+    local handle, err = internal.popen_create(command, flags)
+    if err ~= nil then
+        return nil, err
+    end
+
+    local popen_handle = {
+        -- a handle itself for future use
+        popen_handle = handle,
+
+        -- sleeping period for the @wait method
+        wait_secs = 1,
+
+        -- size of a read buffer to allocate
+        -- in case of implicit read
+        read_buf_size = 512,
+    }
+
+    setmetatable(popen_handle, popen_mt)
+    return popen_handle
+end
+
 return fio
-- 
2.20.1

[Tarantool-patches] [PATCH 5/5] test: Add app/popen test

[Cyrill Gorcunov]

To test basic functionality of popen code.

Fixes #4031

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 test/app/popen.result   | 105 ++++++++++++++++++++++++++++++++++++++++
 test/app/popen.test.lua |  39 +++++++++++++++
 2 files changed, 144 insertions(+)
 create mode 100644 test/app/popen.result
 create mode 100644 test/app/popen.test.lua

diff --git a/test/app/popen.result b/test/app/popen.result
new file mode 100644
index 000000000..1a27b1b0c
--- /dev/null
+++ b/test/app/popen.result
@@ -0,0 +1,105 @@
+-- test-run result file version 2
+fio = require('fio')
+ | ---
+ | ...
+test_run = require('test_run').new()
+ | ---
+ | ...
+
+local err, reason, exit_code
+ | ---
+ | ...
+
+--
+-- Trivial echo output
+--
+script="echo -n 1 2 3 4 5"
+ | ---
+ | ...
+popen = fio.popen(script, "r")
+ | ---
+ | ...
+popen:wait()
+ | ---
+ | - null
+ | - 1
+ | - 0
+ | ...
+popen:read()
+ | ---
+ | - 1 2 3 4 5
+ | ...
+popen:close()
+ | ---
+ | - true
+ | - null
+ | ...
+
+--
+-- Test info and killing of a child process
+--
+script="while [ 1 ]; do sleep 10; done"
+ | ---
+ | ...
+popen = fio.popen(script, "r")
+ | ---
+ | ...
+popen:kill()
+ | ---
+ | - true
+ | ...
+--
+-- Killing child may be queued and depends on
+-- system load, so we may get ESRCH here.
+err, reason, exit_code = popen:wait()
+ | ---
+ | ...
+popen:status()
+ | ---
+ | - null
+ | - 2
+ | - 9
+ | ...
+info = popen:info()
+ | ---
+ | ...
+info["state"]
+ | ---
+ | - exited
+ | ...
+info["flags"]
+ | ---
+ | - 834
+ | ...
+info["exit_code"]
+ | ---
+ | - 9
+ | ...
+popen:close()
+ | ---
+ | - true
+ | - null
+ | ...
+
+--
+-- Test for stdin/out stream
+--
+script="prompt=''; read -n 5 prompt; echo -n $prompt"
+ | ---
+ | ...
+popen = fio.popen(script, "rw")
+ | ---
+ | ...
+popen:write("input")
+ | ---
+ | - 5
+ | ...
+popen:read()
+ | ---
+ | - input
+ | ...
+popen:close()
+ | ---
+ | - true
+ | - null
+ | ...
diff --git a/test/app/popen.test.lua b/test/app/popen.test.lua
new file mode 100644
index 000000000..46c49cb18
--- /dev/null
+++ b/test/app/popen.test.lua
@@ -0,0 +1,39 @@
+fio = require('fio')
+test_run = require('test_run').new()
+
+local err, reason, exit_code
+
+--
+-- Trivial echo output
+--
+script="echo -n 1 2 3 4 5"
+popen = fio.popen(script, "r")
+popen:wait()
+popen:read()
+popen:close()
+
+--
+-- Test info and killing of a child process
+--
+script="while [ 1 ]; do sleep 10; done"
+popen = fio.popen(script, "r")
+popen:kill()
+--
+-- Killing child may be queued and depends on
+-- system load, so we may get ESRCH here.
+err, reason, exit_code = popen:wait()
+popen:status()
+info = popen:info()
+info["state"]
+info["flags"]
+info["exit_code"]
+popen:close()
+
+--
+-- Test for stdin/out stream
+--
+script="prompt=''; read -n 5 prompt; echo -n $prompt"
+popen = fio.popen(script, "rw")
+popen:write("input")
+popen:read()
+popen:close()
-- 
2.20.1

Re: [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine

[Konstantin Osipov]

* Cyrill Gorcunov <gorcunov@gmail.com> [19/11/28 23:46]:
>  - environment variables are flushed to zero, should we provide
>    a way to adjust it (via options) or inherit it instead?

Yes. You can take a look at Python Popen api for inspiration.

>  - popen_kill always send SIGKILL, should not we provide a
>    portable way to customize signal sedning (say symbolic
>    names for signals and pass them here)?

Again, you could take a look at Python - they have kill()
and terminate(), which allows for platform-agnostics hard and soft 
termination. Sending Unix signals to processes can be done by signal()
system call, so you don't need to worry about providing an API
for it in Popen, as long as you expose the child pid in the api.

>  - for native mode we don't do additional processing of arguments
>    thus only plain name of elf executable will be working, we
>    should provide a way for argv explicit passing or
>    do analyze @command for arguments by hands;

>  - need to consider a case where we will be using piping for
>    descriptors (for example we might be writting into stdin
>    of a child from another pipe, for this sake we could use
>    splice() syscall which gonna be a way faster than copying
>    data inside kernel between process). Still the question
>    is -- do we really need it? Since we use interanal flags
>    in popen handle this should not be a big problem to extend
>    this interfaces.

Not in the first version for sure. 
>  	title_free(main_argc, main_argv);
>  
> +	popen_fini();

The convention is to use new/delete for functions which
allocate + initialize and destroy + deallocate an object.

create/destroy for functions which initialize/destroy an object
but do not handle memory management.

init/free for functions which initialize/destroy libraries and
subsystems. Please stick to it.

-- 
Konstantin Osipov, Moscow, Russia

Re: [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine

[Konstantin Osipov]

* Cyrill Gorcunov <gorcunov@gmail.com> [19/11/28 23:46]:
> +/**
> + * popen_write - write data to the child stdin
> + * @handle:	popen handle
> + * @buf:	data to write
> + * @count:	number of bytes to write
> + * @flags:	a flag representing stdin peer
> + *
> + * Returns number of bytes written or -1 on error.
> + */
> +ssize_t
> +popen_write(struct popen_handle *handle, void *buf,
> +	    size_t count, unsigned int flags)
> +{
> +	if (!popen_may_io(handle, STDIN_FILENO, flags))
> +		return -1;
> +
> +	say_debug("popen: %d: write idx %d",
> +		  handle->pid, STDIN_FILENO);
> +
> +	return write(handle->fds[STDIN_FILENO], buf, count);

I don't understand, is this blocking or not?

If this is blocking, where is it supposed to be called from?

Shouldn't you be using a non-blocking I/O like coio does?

Besides, even though I realize it's a pipe, so real world
return values may be different, but write() can return a partial
result even in blocking mode. Why did you choose to design an API
which will require caller to handle partial writes?


-- 
Konstantin Osipov, Moscow, Russia

Re: [Tarantool-patches] [PATCH 2/5] lua/fio: Add lbox_fio_push_error as a separate helper

[Konstantin Osipov]

* Cyrill Gorcunov <gorcunov@gmail.com> [19/11/28 23:46]:
> Since lbox_fio_pushbool always push boolean first
> it is inpossible to use it inside other routines.
> Thus make a separate helper lbox_fio_push_error
> which will be used in popen code later.
> 

OK, now I get why you simply use write() in popen API - you
always wrap all calls to popen with a eio call. But why?

Isn't it sufficient to only wrap vfork, everything else 
can be done in the main thread, what do you think?


-- 
Konstantin Osipov, Moscow, Russia

Re: [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine

[Cyrill Gorcunov]

On Fri, Nov 29, 2019 at 08:59:39AM +0300, Konstantin Osipov wrote:
> * Cyrill Gorcunov <gorcunov@gmail.com> [19/11/28 23:46]:
> > +/**
> > + * popen_write - write data to the child stdin
> > + * @handle:	popen handle
> > + * @buf:	data to write
> > + * @count:	number of bytes to write
> > + * @flags:	a flag representing stdin peer
> > + *
> > + * Returns number of bytes written or -1 on error.
> > + */
> > +ssize_t
> > +popen_write(struct popen_handle *handle, void *buf,
> > +	    size_t count, unsigned int flags)
> > +{
> > +	if (!popen_may_io(handle, STDIN_FILENO, flags))
> > +		return -1;
> > +
> > +	say_debug("popen: %d: write idx %d",
> > +		  handle->pid, STDIN_FILENO);
> > +
> > +	return write(handle->fds[STDIN_FILENO], buf, count);
> 
> I don't understand, is this blocking or not?

It does block "coio" thread, similar to the regular fio.write().

> If this is blocking, where is it supposed to be called from?

It is called from coio, with default priority and wait, 1:1
mapping to fio.write()

> Shouldn't you be using a non-blocking I/O like coio does?

Actually I fear I don't understand this moment -- I wrapped
the popen_write into coio_custom calls, this is what you
mean, or womething else?

> 
> Besides, even though I realize it's a pipe, so real world
> return values may be different, but write() can return a partial
> result even in blocking mode. Why did you choose to design an API
> which will require caller to handle partial writes?

It is first draft to gather comments. I already fixed a similar
issue for regular fio.write() (the patch is flying around) and
will address the partial writes on the next series.

But I think you noticed already that our fio.write() is simply
broken by design -- we do return true/false but instead we should
return bytes written to the caller. It is always up to the caller
what to do with partial writes because only the caller knowns the
context of the call. Can he proceed with partial writes, or he
should abort the whole write and set offset back to the original
position? For this reason exactly the system call return the number
of bytes written not true/false.

Anyway I have to address partial writes.

	Cyrill

Re: [Tarantool-patches] [PATCH 2/5] lua/fio: Add lbox_fio_push_error as a separate helper

[Cyrill Gorcunov]

On Fri, Nov 29, 2019 at 09:02:21AM +0300, Konstantin Osipov wrote:
> * Cyrill Gorcunov <gorcunov@gmail.com> [19/11/28 23:46]:
> > Since lbox_fio_pushbool always push boolean first
> > it is inpossible to use it inside other routines.
> > Thus make a separate helper lbox_fio_push_error
> > which will be used in popen code later.
> > 
> 
> OK, now I get why you simply use write() in popen API - you
> always wrap all calls to popen with a eio call. But why?

Because there could be calls with higher priority. coio
keeps requests in prio-heap (or similar) so we should
follow the scheme and every call to popen should be
dispatched in compare with other calls.

> 
> Isn't it sufficient to only wrap vfork, everything else 
> can be done in the main thread, what do you think?

You mean to call popen_helpers directly? From programming
pov I would love to make it so, the code base would shrink
a lot but I fear this won't be looking as a solid design.
Currently all our i/o goes via coio and scheduled. To be
honest I don't know yet which solution would be better.
Gimme some more time to investigate, I think we could
gather statistics once things settle down and remove
coio calls to popen if we decide. Thanks it is internal
stuff hidden from Lua users and they will not have to
change anything.

	Cyrill

Re: [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine

[Cyrill Gorcunov]

On Fri, Nov 29, 2019 at 08:52:14AM +0300, Konstantin Osipov wrote:
> * Cyrill Gorcunov <gorcunov@gmail.com> [19/11/28 23:46]:
> >  - environment variables are flushed to zero, should we provide
> >    a way to adjust it (via options) or inherit it instead?
> 
> Yes. You can take a look at Python Popen api for inspiration.

Will do, thanks!

> 
> >  - popen_kill always send SIGKILL, should not we provide a
> >    portable way to customize signal sedning (say symbolic
> >    names for signals and pass them here)?
> 
> Again, you could take a look at Python - they have kill()
> and terminate(), which allows for platform-agnostics hard and soft 
> termination. Sending Unix signals to processes can be done by signal()
> system call, so you don't need to worry about providing an API
> for it in Popen, as long as you expose the child pid in the api.
> 
> >  - for native mode we don't do additional processing of arguments
> >    thus only plain name of elf executable will be working, we
> >    should provide a way for argv explicit passing or
> >    do analyze @command for arguments by hands;
> 
> >  - need to consider a case where we will be using piping for
> >    descriptors (for example we might be writting into stdin
> >    of a child from another pipe, for this sake we could use
> >    splice() syscall which gonna be a way faster than copying
> >    data inside kernel between process). Still the question
> >    is -- do we really need it? Since we use interanal flags
> >    in popen handle this should not be a big problem to extend
> >    this interfaces.
> 
> Not in the first version for sure. 

Yup. But you know I think of a future enhancement, and if I not
missing something obvious there still enough place in @flags
so we will be able to mark every stdX as a pipe and handle
accordingly.

> >  	title_free(main_argc, main_argv);
> >  
> > +	popen_fini();
> 
> The convention is to use new/delete for functions which
> allocate + initialize and destroy + deallocate an object.
> 
> create/destroy for functions which initialize/destroy an object
> but do not handle memory management.
> 
> init/free for functions which initialize/destroy libraries and
> subsystems. Please stick to it.

Sure, thanks!

	Cyrill

Re: [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine

[Konstantin Osipov]

* Cyrill Gorcunov <gorcunov@gmail.com> [19/11/29 14:06]:
> > Shouldn't you be using a non-blocking I/O like coio does?
> 
> Actually I fear I don't understand this moment -- I wrapped
> the popen_write into coio_custom calls, this is what you
> mean, or womething else?

Yes, I meant it.
there is no need to call a thread to write to a file descriptor.

> But I think you noticed already that our fio.write() is simply
> broken by design -- we do return true/false but instead we should
> return bytes written to the caller. It is always up to the caller
> what to do with partial writes because only the caller knowns the
> context of the call. Can he proceed with partial writes, or he
> should abort the whole write and set offset back to the original
> position? For this reason exactly the system call return the number
> of bytes written not true/false.
> 
> Anyway I have to address partial writes.

fio is more high level than a thin abstraction around syscalls. It
can block until the full amount is written. hence true/false.

Only the sockets in tarantool are low-level wrappers, other apis
aim at doing something useful.

-- 
Konstantin Osipov, Moscow, Russia

Re: [Tarantool-patches] [PATCH 2/5] lua/fio: Add lbox_fio_push_error as a separate helper

[Konstantin Osipov]

* Cyrill Gorcunov <gorcunov@gmail.com> [19/11/29 14:06]:
> > OK, now I get why you simply use write() in popen API - you
> > always wrap all calls to popen with a eio call. But why?
> 
> Because there could be calls with higher priority. coio
> keeps requests in prio-heap (or similar) so we should
> follow the scheme and every call to popen should be
> dispatched in compare with other calls.

I don't think you should bother with this. I don't see any
practical implication of doing it through coio, except
overhead/complexity it brings.

> > Isn't it sufficient to only wrap vfork, everything else 
> > can be done in the main thread, what do you think?
> 
> You mean to call popen_helpers directly? From programming
> pov I would love to make it so, the code base would shrink
> a lot but I fear this won't be looking as a solid design.

> Currently all our i/o goes via coio and scheduled. To be
> honest I don't know yet which solution would be better.

Don't mix up coio and eio please. coio is
cooperative-wihtin-the-same-thread. eio is async via a thread
pool. Eio should be killed altogether now that asyncio in linux is
mature enough, and there is io_uring.
eio is just a legacy from the times when there was no good non-blocking
file io on linux.

> Gimme some more time to investigate, I think we could
> gather statistics once things settle down and remove
> coio calls to popen if we decide. Thanks it is internal
> stuff hidden from Lua users and they will not have to
> change anything.
> 
> 	Cyrill

-- 
Konstantin Osipov, Moscow, Russia

Re: [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine

[Cyrill Gorcunov]

On Fri, Nov 29, 2019 at 02:19:03PM +0300, Konstantin Osipov wrote:
> * Cyrill Gorcunov <gorcunov@gmail.com> [19/11/29 14:06]:
> > > Shouldn't you be using a non-blocking I/O like coio does?
> > 
> > Actually I fear I don't understand this moment -- I wrapped
> > the popen_write into coio_custom calls, this is what you
> > mean, or womething else?
> 
> Yes, I meant it.
> there is no need to call a thread to write to a file descriptor.

And how it is different from fio.write()? I mean I don't understand
why for regular writes in fio.write() we call for coio and for
popen we should not do the same. Again. Kostya, I don't mind
dropping all this coio wrappers except vfork but this make
popen to look very different are you sure it is ok?

> > But I think you noticed already that our fio.write() is simply
> > broken by design -- we do return true/false but instead we should
> > return bytes written to the caller. It is always up to the caller
> > what to do with partial writes because only the caller knowns the
> > context of the call. Can he proceed with partial writes, or he
> > should abort the whole write and set offset back to the original
> > position? For this reason exactly the system call return the number
> > of bytes written not true/false.
> > 
> > Anyway I have to address partial writes.
> 
> fio is more high level than a thin abstraction around syscalls. It
> can block until the full amount is written. hence true/false.

Which is true once my patch https://lists.tarantool.org/pipermail/tarantool-patches/2019-November/012635.html
get merged, without it our fio.write() is simply broken :/
But I see what you mean.

> 
> Only the sockets in tarantool are low-level wrappers,
> other apis aim at doing something useful.

Re: [Tarantool-patches] [PATCH 2/5] lua/fio: Add lbox_fio_push_error as a separate helper

[Cyrill Gorcunov]

On Fri, Nov 29, 2019 at 02:22:03PM +0300, Konstantin Osipov wrote:
> > Because there could be calls with higher priority. coio
> > keeps requests in prio-heap (or similar) so we should
> > follow the scheme and every call to popen should be
> > dispatched in compare with other calls.
> 
> I don't think you should bother with this. I don't see any
> practical implication of doing it through coio, except
> overhead/complexity it brings.

OK, thanks!

> > > Isn't it sufficient to only wrap vfork, everything else 
> > > can be done in the main thread, what do you think?
> > 
> > You mean to call popen_helpers directly? From programming
> > pov I would love to make it so, the code base would shrink
> > a lot but I fear this won't be looking as a solid design.
> 
> > Currently all our i/o goes via coio and scheduled. To be
> > honest I don't know yet which solution would be better.
> 
> Don't mix up coio and eio please. coio is
> cooperative-wihtin-the-same-thread. eio is async via a thread
> pool. Eio should be killed altogether now that asyncio in linux is
> mature enough, and there is io_uring.

OK, so the longterm plans are to get rid of eio and
use io_uring instead? I'm both hands for this kind of
async io, except we will get a huge problems on FreeBSD/MachO
I think (or we will have to keep eio for them).

> eio is just a legacy from the times when there was no good non-blocking
> file io on linux.

Thanks for explanation!

Re: [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine

[Konstantin Osipov]

* Cyrill Gorcunov <gorcunov@gmail.com> [19/11/29 16:31]:
> And how it is different from fio.write()? I mean I don't understand
> why for regular writes in fio.write() we call for coio and for
> popen we should not do the same.

Cyrill, the difference is popen works with a pipe, not a file.
Unix supports non-blocking IO for pipes, and usually it doesn't
support it for files. 

-- 
Konstantin Osipov, Moscow, Russia

Re: [Tarantool-patches] [PATCH 2/5] lua/fio: Add lbox_fio_push_error as a separate helper

[Konstantin Osipov]

* Cyrill Gorcunov <gorcunov@gmail.com> [19/11/29 16:31]:
> > Don't mix up coio and eio please. coio is
> > cooperative-wihtin-the-same-thread. eio is async via a thread
> > pool. Eio should be killed altogether now that asyncio in linux is
> > mature enough, and there is io_uring.
> 
> OK, so the longterm plans are to get rid of eio and
> use io_uring instead? I'm both hands for this kind of
> async io, except we will get a huge problems on FreeBSD/MachO
> I think (or we will have to keep eio for them).

On Mac and FreeBSD one can use aio.


-- 
Konstantin Osipov, Moscow, Russia

Re: [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine

[Cyrill Gorcunov]

On Fri, Nov 29, 2019 at 05:50:28PM +0300, Konstantin Osipov wrote:
> * Cyrill Gorcunov <gorcunov@gmail.com> [19/11/29 16:31]:
> > And how it is different from fio.write()? I mean I don't understand
> > why for regular writes in fio.write() we call for coio and for
> > popen we should not do the same.
> 
> Cyrill, the difference is popen works with a pipe, not a file.
> Unix supports non-blocking IO for pipes, and usually it doesn't
> support it for files. 

OK, i've got what you mean. Kostya, let me express some more details
on current implementation, maybe I simply miss something obvious:

1) The pipes I use are opened in blocking mode, non-blocking read
   implemented via explicit call to poll() with timeout option
   (to be honest I'm a bit worried about signal interruption from
    timers which libev provides, won't they interrupt poll since
    they all are living in same thread, I need to understand this
    moment later).

   IOW, using pipes in blocking mode and poll with timeout for
   nonblocking read is correct solution or we shoudl use nonbloking
   ops from the very beginning?

2) When I do various ops on popen object (say sending kill, fetching
   status of a process and etc) I block SIGCHLD of coio thread,
   otherwise there is a race with external users which could simply
   kill the "command" process we're running and popen->pid no longer
   valid, what is worse someone else could be take this pid already.

   Thus I need to block signals for this sake, and now if I start
   calling the popen helpers without entering coio thread (ie without
   coio_custom helpers) I wont be able to block signals. If I understand
   correctly the console is running inside own thread, no?

Re: [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine

[Cyrill Gorcunov]

On Fri, Nov 29, 2019 at 06:14:10PM +0300, Cyrill Gorcunov wrote:
> 
> 2) When I do various ops on popen object (say sending kill, fetching
>    status of a process and etc) I block SIGCHLD of coio thread,
>    otherwise there is a race with external users which could simply
>    kill the "command" process we're running and popen->pid no longer
>    valid, what is worse someone else could be take this pid already.
> 
>    Thus I need to block signals for this sake, and now if I start
>    calling the popen helpers without entering coio thread (ie without
>    coio_custom helpers) I wont be able to block signals. If I understand
>    correctly the console is running inside own thread, no?

Just realized that this should be fine since we're sharing signals.
Drop the question 2.

	Cyrill

Re: [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine

[Konstantin Osipov]

* Cyrill Gorcunov <gorcunov@gmail.com> [19/11/29 18:18]:
> > Cyrill, the difference is popen works with a pipe, not a file.
> > Unix supports non-blocking IO for pipes, and usually it doesn't
> > support it for files. 
> 
> OK, i've got what you mean. Kostya, let me express some more details
> on current implementation, maybe I simply miss something obvious:
> 
> 1) The pipes I use are opened in blocking mode, non-blocking read
>    implemented via explicit call to poll() with timeout option
>    (to be honest I'm a bit worried about signal interruption from
>     timers which libev provides, won't they interrupt poll since
>     they all are living in same thread, I need to understand this
>     moment later).
> 
>    IOW, using pipes in blocking mode and poll with timeout for
>    nonblocking read is correct solution or we shoudl use nonbloking
>    ops from the very beginning?

I suggest using non-blocking IO.

> 2) When I do various ops on popen object (say sending kill, fetching
>    status of a process and etc) I block SIGCHLD of coio thread,

Let's call this *eio* thread, please. coio is co-operative io. eio is
thread-pool-based-io. eio API was in coeio namespace first, later I moved it
to coio namespace.

>    otherwise there is a race with external users which could simply
>    kill the "command" process we're running and popen->pid no longer
>    valid, what is worse someone else could be take this pid already.

We discussed this and pid reuse is impossible unless you collect
the status of a child. You can easily mark the handle as dead as
soon as you get sigchild and collect it. I don't see any issue
here. 
> 
>    Thus I need to block signals for this sake, and now if I start
>    calling the popen helpers without entering coio thread (ie without
>    coio_custom helpers) I wont be able to block signals. If I understand
>    correctly the console is running inside own thread, no?

I don't understand this idea of blocking signals. You can't
control signal masks of all tarantool threads, so what's the point
of blocking a signal in a single thread anyway? It will get
delivered to a different thread in the same process. 

libev handles the signal masks for you already. You should do
nothing about it - just install the child handler and let it work
for you.

Or else I don't understand what you're trying to accomplish.


-- 
Konstantin Osipov, Moscow, Russia

Re: [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine

[Cyrill Gorcunov]

On Fri, Nov 29, 2019 at 09:31:44PM +0300, Konstantin Osipov wrote:
> > 
> >    IOW, using pipes in blocking mode and poll with timeout for
> >    nonblocking read is correct solution or we shoudl use nonbloking
> >    ops from the very beginning?
> 
> I suggest using non-blocking IO.

Once we sart using non-blocking IO the read() could return -EAGAIN.
I think I need to find out how python is handling this situation,
is their read is blocking or not.

> > 2) When I do various ops on popen object (say sending kill, fetching
> >    status of a process and etc) I block SIGCHLD of coio thread,
> 
> Let's call this *eio* thread, please. coio is co-operative io. eio is
> thread-pool-based-io. eio API was in coeio namespace first, later
> I moved it to coio namespace.

I call it coio simply because it is the name of the thread.

> 
> >    otherwise there is a race with external users which could simply
> >    kill the "command" process we're running and popen->pid no longer
> >    valid, what is worse someone else could be take this pid already.
> 
> We discussed this and pid reuse is impossible unless you collect
> the status of a child. You can easily mark the handle as dead as
> soon as you get sigchild and collect it. I don't see any issue
> here. 

The eio reaps children itself, ie calls for wait. Thus imagine a situation,
we start killing the process like

popen_kill(handle)
	...
	kill(handle->pid)
	...

but before we reach kill() this process exited by self or killed
by a user on the node. The signal handler sets pid = -1 and we
call kill(-1). Which is wrong of course.

> > 
> >    Thus I need to block signals for this sake, and now if I start
> >    calling the popen helpers without entering coio thread (ie without
> >    coio_custom helpers) I wont be able to block signals. If I understand
> >    correctly the console is running inside own thread, no?
> 
> I don't understand this idea of blocking signals. You can't
> control signal masks of all tarantool threads, so what's the point
> of blocking a signal in a single thread anyway? It will get
> delivered to a different thread in the same process.
> 
> libev handles the signal masks for you already. You should do
> nothing about it - just install the child handler and let it work
> for you.

I must confess I simply forget that SIGCHLD (when program get terminated)
is sent by the kernel as a group signal into shared pending signals queue,
what a shame :/ So blocking signals won't work here. But I have to order
"handle->pid" access somehow so it would be either valid or not,
at least for popen_kill(). We can't use mutexes or similar in signal
handler. Need to think...

Re: [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine

[Cyrill Gorcunov]

On Fri, Nov 29, 2019 at 10:17:08PM +0300, Cyrill Gorcunov wrote:
> > 
> > We discussed this and pid reuse is impossible unless you collect
> > the status of a child. You can easily mark the handle as dead as
> > soon as you get sigchild and collect it. I don't see any issue
> > here. 
> 
> The eio reaps children itself, ie calls for wait. Thus imagine a situation,
> we start killing the process like
> 
> popen_kill(handle)
> 	...
> 	kill(handle->pid)
> 	...
> 
> but before we reach kill() this process exited by self or killed
> by a user on the node. The signal handler sets pid = -1 and we
> call kill(-1). Which is wrong of course.

You know, without signal blocking I fear we simply won't be able
to tack children properly. Look here is an example

	pid = vfork();
	if (pid == 0) {
		_exit(1);

		here sigchld already delivered to the libev
		and it reaped it, vanishing from the system
		so that anyother application can reuse it
	} else {
		ev_child_init(&handle->ev_sigchld, ev_sigchld_cb, pid, 0);
		ev_child_start(EV_DEFAULT_ &handle->ev_sigchld);

		but pid already dead and reused by someone else,
		as I said wait() called under the hood, we simply
		don't control it
	}

and without signal blocking we can't order pid livetime anyhow.
You know in my first versions I setup ev_child_init with pid=0
and been expecting the sgnal handler catches _any_ sigchld but
it didn't work :/ Thus early _exit(1) in child process was
simply invisible to the rest of tarantool code.

Re: [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine

[Konstantin Osipov]

* Cyrill Gorcunov <gorcunov@gmail.com> [19/11/30 07:03]:
> Once we sart using non-blocking IO the read() could return -EAGAIN.
> I think I need to find out how python is handling this situation,
> is their read is blocking or not.

Take a look at how coio works. It adds the descriptor to the event
loop and yields the current fiber.

> 
> The eio reaps children itself, ie calls for wait. Thus imagine a situation,
> we start killing the process like
> 
> popen_kill(handle)
> 	...
> 	kill(handle->pid)
> 	...
> 
> but before we reach kill() this process exited by self or killed
> by a user on the node. The signal handler sets pid = -1 and we
> call kill(-1). Which is wrong of course.

Can't you check the pid > 0 before you send the signal?

-- 
Konstantin Osipov, Moscow, Russia

Re: [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine

[Konstantin Osipov]

* Cyrill Gorcunov <gorcunov@gmail.com> [19/11/30 07:03]:
> > but before we reach kill() this process exited by self or killed
> > by a user on the node. The signal handler sets pid = -1 and we
> > call kill(-1). Which is wrong of course.
> 
> You know, without signal blocking I fear we simply won't be able
> to tack children properly. Look here is an example

First of all, thanks for the explanation - now I understand the
problem at least.

> 	pid = vfork();
> 	if (pid == 0) {
> 		_exit(1);
> 
> 		here sigchld already delivered to the libev
> 		and it reaped it, vanishing from the system
> 		so that anyother application can reuse it

No, this is not how it works. Yes, the signal itself *may* 
be delivered asynchronously (*sometimes*), but the callback is called
synchronously. In other words, when you do a vfork()
and check the non-zero pid return value you're executing an
existing libev callback. No other callback will get invoked until
you finish with your callback.

Even though it's an implementation detail, the signal is *also*
delivered synchronously on Linux. It is sent to signal_fd, and 
will be processed only upon the next iteration of the event loop,
when all activated callbacks are finished. 

So you can safely assume that your pid will not get lost.
And you should not assume - you should test it.

> 	} else {
> 		ev_child_init(&handle->ev_sigchld, ev_sigchld_cb, pid, 0);
> 		ev_child_start(EV_DEFAULT_ &handle->ev_sigchld);
> 
> 		but pid already dead and reused by someone else,
> 		as I said wait() called under the hood, we simply
> 		don't control it
> 	}
> 
> and without signal blocking we can't order pid livetime anyhow.
> You know in my first versions I setup ev_child_init with pid=0
> and been expecting the sgnal handler catches _any_ sigchld but
> it didn't work :/ Thus early _exit(1) in child process was
> simply invisible to the rest of tarantool code.


-- 
Konstantin Osipov, Moscow, Russia

Re: [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine

[Cyrill Gorcunov]

On Sat, Nov 30, 2019 at 07:14:05AM +0300, Konstantin Osipov wrote:
> * Cyrill Gorcunov <gorcunov@gmail.com> [19/11/30 07:03]:
> > Once we sart using non-blocking IO the read() could return -EAGAIN.
> > I think I need to find out how python is handling this situation,
> > is their read is blocking or not.
> 
> Take a look at how coio works. It adds the descriptor to the event
> loop and yields the current fiber.

I will, thanks! You know there is another problem with nonblocking
descriptors: consider a case where user runs a script like in my
test "input=''; read -n 5 input; echo $input". If you run it inside
a regular terminal the script will wait for input to apprear first,
but if we provide nonblocking pipe the "read" will exit with
-EAGAIN and script fail. Actually my first implementations have been
creating pipes with O_NONBLOCK and since such test case start to fail
I dropped O_NONBLOCK then.

> > The eio reaps children itself, ie calls for wait. Thus imagine a situation,
> > we start killing the process like
> > 
> > popen_kill(handle)
> > 	...
> > 	kill(handle->pid)
> > 	...
> > 
> > but before we reach kill() this process exited by self or killed
> > by a user on the node. The signal handler sets pid = -1 and we
> > call kill(-1). Which is wrong of course.
> 
> Can't you check the pid > 0 before you send the signal?

This won't work, a signal can interrupt us in any moment and
set the pid to -1 between if() and kill().

Re: [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine

[Cyrill Gorcunov]

On Sat, Nov 30, 2019 at 07:21:58AM +0300, Konstantin Osipov wrote:
> First of all, thanks for the explanation - now I understand the
> problem at least.
> 
> > 	pid = vfork();
> > 	if (pid == 0) {
> > 		_exit(1);
> > 
> > 		here sigchld already delivered to the libev
> > 		and it reaped it, vanishing from the system
> > 		so that anyother application can reuse it
> 
> No, this is not how it works. Yes, the signal itself *may* 
> be delivered asynchronously (*sometimes*), but the callback is called
> synchronously. In other words, when you do a vfork()
> and check the non-zero pid return value you're executing an
> existing libev callback. No other callback will get invoked until
> you finish with your callback.

You know, I think I need to read more libev source code first
to be able to comment. I'll back later. Kostya, thanks a huge
about all your participation, I really appreciate!

Re: [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine

[Konstantin Osipov]

* Cyrill Gorcunov <gorcunov@gmail.com> [19/11/30 10:39]:
> > * Cyrill Gorcunov <gorcunov@gmail.com> [19/11/30 07:03]:
> > > Once we sart using non-blocking IO the read() could return -EAGAIN.
> > > I think I need to find out how python is handling this situation,
> > > is their read is blocking or not.
> > 
> > Take a look at how coio works. It adds the descriptor to the event
> > loop and yields the current fiber.
> 
> I will, thanks! You know there is another problem with nonblocking
> descriptors: consider a case where user runs a script like in my
> test "input=''; read -n 5 input; echo $input". If you run it inside
> a regular terminal the script will wait for input to apprear first,
> but if we provide nonblocking pipe the "read" will exit with
> -EAGAIN and script fail. Actually my first implementations have been
> creating pipes with O_NONBLOCK and since such test case start to fail
> I dropped O_NONBLOCK then.

Could you provide a Lua example usage of popen() which would lead 
to the situation you describe here?

Seems like in order for that to happen the Lua script has to
provide both input and output streams to the called program, and
in this case the script should not expect that there is any
ordering in which the input is consumed and the output is
produced.

In any case discussing this problem would be easier if there is an
example.

-- 
Konstantin Osipov, Moscow, Russia

Re: [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine

[Cyrill Gorcunov]

On Sat, Nov 30, 2019 at 01:04:45PM +0300, Konstantin Osipov wrote:
> 
> Could you provide a Lua example usage of popen() which would lead 
> to the situation you describe here?
> 
> Seems like in order for that to happen the Lua script has to
> provide both input and output streams to the called program, and
> in this case the script should not expect that there is any
> ordering in which the input is consumed and the output is
> produced.
> 
> In any case discussing this problem would be easier if there is an
> example.

An example is in one of my patch, here is it (result file)
---

--
-- Test for stdin/out stream
--
script="prompt=''; read -n 5 prompt; echo -n $prompt"
 | ---
 | ...
popen = fio.popen(script, "rw")
 | ---
 | ...
popen:write("input")
 | ---
 | - 5
 | ...
popen:read()
 | ---
 | - input
 | ...
popen:close()
 | ---
 | - true
 | - null
 | ...

The main thing is that as far as I remember "real" stdin/out/err are
the unix pty terminals, and they are in blocking mode. IOW if you
run

 $ sh -c "prompt=''; read -n 5 prompt; echo -n $prompt"

inside real shell the read will be blocked until you enter data
to stdin. And I think users of popen will expect the same behaviour.
Anyway, Kostya, I'll investigate python behaviour more on the weekend
to get in with more details.

Re: [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine

[Cyrill Gorcunov]

On Sat, Nov 30, 2019 at 01:47:34PM +0300, Cyrill Gorcunov wrote:
> On Sat, Nov 30, 2019 at 01:04:45PM +0300, Konstantin Osipov wrote:
> > 
> > Could you provide a Lua example usage of popen() which would lead 
> > to the situation you describe here?
> > 
> > Seems like in order for that to happen the Lua script has to
> > provide both input and output streams to the called program, and
> > in this case the script should not expect that there is any
> > ordering in which the input is consumed and the output is
> > produced.
> > 
> > In any case discussing this problem would be easier if there is an
> > example.
> 
> An example is in one of my patch, here is it (result file)
> ---
...
>  $ sh -c "prompt=''; read -n 5 prompt; echo -n $prompt"

Forgot to mention that the example in previous message is
for pipe opened in blocked mode. If o_nonblock used the
script fails in "read" action. I think the main target now
for me is to investigate which mode uses python in subprocess
module and etc (simply because users are familiar with it and
will expect us to behave the same I think).

Re: [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine

[Cyrill Gorcunov]

On Sat, Nov 30, 2019 at 01:54:54PM +0300, Cyrill Gorcunov wrote:
> 
> Forgot to mention that the example in previous message is
> for pipe opened in blocked mode. If o_nonblock used the
> script fails in "read" action. I think the main target now
> for me is to investigate which mode uses python in subprocess
> module and etc (simply because users are familiar with it and
> will expect us to behave the same I think).

Python opens pipes in blocked mode
--- sub.py
import subprocess

args=["sh", "-c", "prompt=''; read -n 5 prompt; echo $prompt"]
process = subprocess.Popen(args, stdout=subprocess.PIPE)
(stdout_data, stderr_data) = process.communicate()
print(stdout_data)
---

$ python3 sub.py
12345b'12345\n'

So it waited for me to type "12345" on keyboard and echo'ed it back.

If we look into strace output we will see

 | pipe2([3, 4], O_CLOEXEC)          = 0

so I think we should follow the convention and open
pipes in blocking mode. Still we could provide an
option to open in nonblocking mode if user wants.

	Cyrill

Re: [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine

[Konstantin Osipov]

* Cyrill Gorcunov <gorcunov@gmail.com> [19/11/30 23:03]:
> Forgot to mention that the example in previous message is
> for pipe opened in blocked mode. If o_nonblock used the
> script fails in "read" action. I think the main target now
> for me is to investigate which mode uses python in subprocess
> module and etc (simply because users are familiar with it and
> will expect us to behave the same I think).

It will fail unless you handle EAGAIN and yield. Do you?

-- 
Konstantin Osipov, Moscow, Russia

Re: [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine

[Cyrill Gorcunov]

On Sat, Nov 30, 2019 at 11:30:54PM +0300, Konstantin Osipov wrote:
> * Cyrill Gorcunov <gorcunov@gmail.com> [19/11/30 23:03]:
> > Forgot to mention that the example in previous message is
> > for pipe opened in blocked mode. If o_nonblock used the
> > script fails in "read" action. I think the main target now
> > for me is to investigate which mode uses python in subprocess
> > module and etc (simply because users are familiar with it and
> > will expect us to behave the same I think).
> 
> It will fail unless you handle EAGAIN and yield. Do you?

True. If I handle -EAGAIN inside my test-script it will work,
but this is not a standart behaviour users expect I think. As
I pointed in another message the python subprocess opens pipe
in blocking mode and I suppose we should do the same, otherwise
users will have to keep in mind that our pipes are opened in
nonblocking mode, which is inconvenient.

Re: [Tarantool-patches] [PATCH 1/5] popen: Introduce a backend engine

[Alexander Turenko]

On Thu, Nov 28, 2019 at 11:45:08PM +0300, Cyrill Gorcunov wrote:
> In the patch we introduce popen backend engine which provides
> a way to execute external programs and communicate with their
> stdin/stdout/stderr streams.

I'll past common notes here (in v1 patchset), because a discussion is
already started here.

CCed Igor Munkin. As far as I know he also shows into this patchset.

----

I propose to split the discussion into the following threads:

- fork/exec/signals (subtopic: Lua API)
- io (subtopic: Lua API)
- other (say, whether to use our diagnotics area or just errno in the
  backend)

IO
--

(For ones who has better knowledge around tarantool code base then me:
skip this section and read 'Pre-proposal for Lua API for our pipes';
this is more a bunch of notes I made for myself re IO APIs we have, but
they can be useful for someone else.)

Let's discuss the latter topic. I'll return to the first one later.

We have several libraries / primitives related to input / output: sio,
libev / coio, eio. I'm not very familiar with them, just looked around
docs and sources and tried to organize my understanding. Please, correct
me if misunderstood some details.

## sio

sio is the very thin wrapper around BSD sockets, which exposes errors
using our diagnostics area (rather then errno) and draws a line between
EWOULDBLOCK (and similar) errors from non-transient ones.

We basically interested in sio_read(), sio_write() and sio_wouldblock().

## libev

libev deserves its own description, but in context of this discussion it
is a wrapper around epoll() that allows to register a callback that will
be called when a file descriptor becomes readable or writeable (or a
timeout occurs).

We basically use it in this way (more complex examples are iproto and
swim):

 | // Get a cord local (means usually thread local) event loop.
 | struct ev_loop *loop = loop();
 | // Initialize a watcher.
 | struct ev_io watcher;
 | ev_io_init(&watcher, callback, fd, EV_READ); // or EV_WRITE, or both
 | // Add the file descriptor to epoll.
 | ev_io_start(loop, &watcher);

 | // In callback (reading for example).
 | int rc = sio_read(<...>);
 | if (rc < 0 && ! sio_wouldblock(errno)) {
 |         // handle an error
 | } else if (rc == 0) { // EOF
 |         ev_io_stop(loop, &watcher);
 |         // handle EOF
 | } else {
 |         // handle actual read
 | }

So it is natural to use libev + sio for a nonblocking file descriptor to
work with it asynchronously, I would even say in event based way.

We can also start a watcher on demand, when sio_wouldblock() returns
true.

## coio (parts that do not use a thread pool under hood)

We however need more: integrate our reads and writes with fibers to
yield until an event (readability, writeability or timeout) occurs. It
seems that coio_*() functions are about this.

We have two functions for reading and writing and bunch of simple
wrappers around them: coio_read_ahead_timeout() and
coio_writev_timeout().

They stitch sio_*() and ev_io_*() functions to perform reads and writes.
When an io operation would block, they registers a watcher and yields
currect fiber. The watcher callback just schedules the fiber back.

It also worth to mention coio_wait(fd, events, timeout), which yields
until the file descriptor becomes readable / writeable (as asked in
'events') or the timeout occurs.

We widely use this API, but it worth to highlight fio and socket Lua
modules as examples.

## libeio + coio_task.h + coio_file.h

It is basically a thread pool that allows to execute a task
asynchronously.

We integrate it with our fibers in coio_task.h API that provides general
purpose coio_call() to perform arbitrary call in a thread (and yield
until it'll end) and coio_getaddrinfo() to perform async DNS resolving.

We also have a bunch of file oriented coio_*() wrappers that calls eio
and yield until an operation ends (see coio_file.h).

Here we potentially interested in coio_read() and coio_write().

However my understanding is that this API is more for file IO rather
then pipes, because thread pool looks as overkill to perform async read
or write of a pipe. I propose to consider coio_read_ahead_timeout(),
coio_writev_timeout() (or wrappers) and coio_wait() to work with pipes.

I looked at libuv docs, and they use its thread pool only for file operations,
getaddrinfo() and user provided tasks:
http://docs.libuv.org/en/v1.x/design.html

Aside of that I was wondered why we even need multiple threads to
perform file IO better and ends with the guess that a disc controller
may schedule operations better, when there are more requested operations
in-fly, while reads and writes that got back to us with EWOULDBLOCK are
not 'in-fly' in this sense (a process may decided to don't retry them).

So, we need more threads just to block more writes (or reads) when
working with files :)

My source is just this SO answer: https://stackoverflow.com/a/8157313/1598057
and my rich imagination.

Pre-proposal for Lua API for our pipes
--------------------------------------

I want to share some kind of sketchy notes around API for parent ends of
stdin / stdout / stderr pipes.

It seems that with our coio functions (ones that are not backed with
libeio) we can just expose file descriptors in non-blocking mode from
our backend and everything else can be handled in a quite simple way
from, say, 'pipe' Lua module (the name is to be discussed).

This drive us to the following points:

* Backend just set fds as non-blocking and expose them. No read / write
  code are needed at all.
* We can expose such file descriptor in Lua as 'pipe' object (or, maybe
  better, as 'reading stream' and 'writing stream' objects).
* We need to call coio_*() read/write/wait functions Lua API
  implementation and this is basically all.

I propose to implement relatively high-level API for read:

* For binary protocols a user likely want to wait until N bytes will
  arrive (or timeout occurs).
* For text protocol a user likely want to read until a delimiter (say a
  newline), or until a timeout; the delimiter can be multibyte.

This more or less reflects socket_object:read() API, see
https://www.tarantool.io/en/doc/2.2/reference/reference_lua/socket/#socket-read

Don't sure however that the API should be exactly same; need to dig a bit more
and understand it better, the doc is quite shetchy.

Maybe we even can extract relevant parts of the code and use them for
read streams and sockets both. It looks natural, because a socket is the
case of a read stream.

tarantool/luatest offers a shortcut to start a fiber for reading from a
pipe. We should consider such shortcut too for running interactive
commands (I mean, to read and write 'simultaneously').

We should consider also to allow a user to pass a buffer optionally as
in fio_handle:read() (and return a Lua string if it is not passed). This
provides the way to eliminate allocating of Lua strings for each read
and so perform reads in more performant way.

----

I'll return to process handles and related Lua API separately. There are
some thoughts around too :)

WBR, Alexander Turenko.