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

[Cyrill Gorcunov <gorcunov@gmail.com>]

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...