error when using with OpenResty cli

[Tieske]

in function get_cur_dir I'm getting this error (I isolated it to the resty cli tool):

$ resty -e "local x = io.popen('pwd 2>/dev/null', 'r'); print(x:read())"
/Users/thijs/code/lua-resty-dns-client
$ resty -e "local x = io.popen('pwd 2>/dev/null', 'r'); print(x:read('*a'))"
nilInterrupted system call4
$

Somehow read("*a") from io.popen triggers the error, where read() succeeds

[hishamhm]

This is an issue with the resty cli. The nginx event loop sends Unix signals and the Lua io library doesn't know about them (because it is OS-agnostic), so the underlying syscall used by io.popen halts with EINTR, and you get the error.

Given that resty is embedding Lua(JIT) in its signal-enabled Unix environment, it's their responsibility to ensure that the Lua they offer works correctly with the environment they're embedding it in. I believe @thibaultcha even sent a patch to OpenResty's repo of LuaJIT adding support for catching EINTR and retrying the C library call in (some?) io calls, which alleviates this issue. I'm afraid it breaks the ability to do Ctrl-C reliably, though. I'm not sure a fully clean solution to this is possible, signals are a pretty "meh" aspect of the Unix design.

In any case, this is not a luacov issue, so I think we can safely close this one. :)

[Tieske]

Yes obviously that is the case. My point would be that we do not need "*a", just reading the first line (default behaviour of read iirc) should be enough.

[thibaultcha]

There is another series of patches addressing this (we reverted the retry on EINTR patch in LuaJIT). See openresty/lua-nginx-module#1296 and other linked PRs.

While the retry on EINTR patch could have been an issue in other applications embedding LuaJIT (I don't really think so), the SA_RESTART flag does not prevent the correct handling of SIGINT within Nginx as far as I remember.

[mpeterv]

If the workaround in luacov is so small and still reliable I think it's worth applying.

[hishamhm]

@thibaultcha thanks for the update! that does sound like a good approach indeed.

@mpeterv @Tieske I don't think this is a reliable solution — it makes the problem less likely to hit, but I think it's still prone to EINTR. Still, the change is harmless so why not.