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

[Alexander Turenko]

On Tue, Jun 04, 2019 at 10:59:31AM +0300, Vladimir Davydov wrote:
> On Tue, Jun 04, 2019 at 10:11:15AM +0300, Stanislav Zudin wrote:
> > 
> > 
> > On 03.06.2019 20:25, Alexander Turenko wrote:
> > > > > Come to think of it, we might need an explicit knob to destroy an object
> > > > > even before it's collected. May be, consider re-adding close() for this?
> > > > > Please consult with Alexander and/or Georgy re this.
> > > > > 
> > > > 
> > > > Sasha, George, what do you think?
> > > 
> > > I'm a bit out of context. Why we need explicit close()? To control
> > > reaching a file descriptor limit? It can be worked around with
> > > `handle = nil collectgarbage()`, but an explicit close() looks better.
> > > Maybe it worth to add.
> > > 
> > 
> > For now there is wait() as a final method. It waits for process termination
> > and releases resources.
> 
> We might want to read process output after waiting for it to complete.
> Or it doesn't make any sense?

I imagine this in the following way.

One way to implement the API is to map C/Unix API to work with child
processes / pipes in more or less direct way and so wait() just waits
for a process to terminate, read() / write() performs operations on a
pipe internal buffer. The only significant differences are proper
integration with our event loop and providing helpful popen handle that
accumulate all related file descriptors and information fields.

Another way is to hide deeper real process and pipes under our
abstractions: wait() expects both a process termination and pipes
closing, while read() / write() performs operations on buffers in our
memory and hide real read() / write(), which is performed in background.
This way is safer in the sense of a pipe buffer size exceeding, but
provides less control over using memory for buffers (they can take a
large memory).

If we'll going the first way it seems natural to allow to do read()
after wait() and provide close() to free resources manually. The second
way allows to read everything to the end in wait() and only then really
close pipes. Both ways should allow a user to perform API's read() after
wait(), the difference only when we actually call close() on a pipe.

I don't see a reason to make things more complex then they are and I
would consider popen implementation as the thin layer between Lua and
Unix API.

But maybe I mix aspects of the API that should be considered separately,
don't sure.

> 
> > So the question was: should we add close() to release resources
> > explicitly?
> > Anyway there is a finalizer who performs a final cleanup for the case when
> > user forgot to do it.
> 
> To trigger the finalizer, we need to delete the variable
> 
>   p = nil
> 
> which isn't very user friendly IMO.

I think explicit close() is needed for applications that want to
carefully manage system resources. Say, you writing a harness (runner)
for tests and each run calls popen. You can run hundreds processes per
second and GC may be not aggressive enough to save you from, say, open
file descriptor limit. One can tune GC options or manually call
collectgarbage(), but is looks more as the workaround rather then
solutions for the problem.