tarantool / tarantool / 8818369172

88%

Build Type

push

github

Committed by Totktonada

Commit Message

popen: add timeout for :wait()

This commit solves several problems:

* Eliminates polling with fiber sleeps for a process status in `:wait()`.
  Now the method waits for libev's SIGCHLD watcher (via a fiber cond).
* Fixes use-after-free and crash/infinite hang in `:wait()` when the
  handle is closed from another fiber.
* Adds `timeout` parameter to `:wait()`.

Popen handles are not reference counted, so the code that waits for a
process completion needs to be a bit tricky to don't access possibly
freed memory. I guess things would be simpler if we implemented
refcounting on the handles, but the same set of problems are generally
solved on the lua/popen side (it tracks `:close()` calls), and I don't
see enough motivation to rearrange it. At least, until we'll create the
handles not only from Lua.

Fixes #4915
Fixes #7653
Fixes #4916

@TarantoolBot document
Title: popen: :wait() now has the timeout parameter

Usage example:

```lua
local ph = popen.new(<...>)
local res, err = ph:wait({timeout = 1})

if res == nil then
    -- Timeout is reached.
    assert(err.type == 'TimedOut')
    <...>
end
```

Also `:wait()` now has defined behavior when the popen handle is closed
from another fiber: the method returns the `ChannelIsClosed` error.

Both updates should have 'Since X.Y.Z' marks in the documentation to
allow users to decide whether to use the new features based on what
tarantool releases should be supported by the calling code. IOW, a user
may lean on the defined close-during-wait behavior or decide to don't.
The same is true for the new timeout option.

See the `lbox_popen_wait()` comment for the updated formal description
of the `<popen handle>:wait(<...>)` method.

Run Details

67394 of 119817 branches covered (56.25%)

46 of 46 new or added lines in 2 files covered. (100.0%)

32 existing lines in 10 files now uncovered.

100129 of 114971 relevant lines covered (87.09%)

2434381.09 hits per line