Re: [Tarantool-patches] [PATCH 1/1] app: handle concatenated argv name-value correctly

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Hi! Thanks for the review!

On 19/02/2020 09:54, Igor Munkin wrote:
> Vlad,
> 
> Thanks for the patch it LGTM.
> 
> However, the Travis job failed on the newly added test[1]. It looks like
> the wait underneath the pclose has failed. I can't reproduce the problem
> on my machine, but I guess the strace output for the failed child:close
> might shed some light on the issue. Nevertheless, what do you think
> regarding omitting the close call since the resourses will be
> automatically released considering the doc[2]?

It is good that you noticed. The problem has nothing to do with Lua as
I found. It is because of libev, which automatically calls wait() every
time when a SIGCHILD is received. We compile with EV_CHILD_ENABLE 1,
which activates that behaviour, and breaks Lua popen. I am not sure
whether it is a bug. But yeah, we can omit :close() I suppose. Then it
will work regardless of EV_CHILD_ENABLE. We probably will need to
disable or properly handle that flag for our own popen.

I changed the test:

================================================================================

diff --git a/test/app/gh-4775-crash-args-l-e.result b/test/app/gh-4775-crash-args-l-e.result
index eff1ee763..88169f700 100644
--- a/test/app/gh-4775-crash-args-l-e.result
+++ b/test/app/gh-4775-crash-args-l-e.result
@@ -9,7 +9,6 @@ child:read()
  | ---
  | - '100'
  | ...
-child:close()
- | ---
- | - true
- | ...
+-- :close() is omitted, because SIGCHILD may be handled by
+-- libev instead of Lua. In that case :close() with fail with
+-- ECHILD, but it does not matter for this test.
diff --git a/test/app/gh-4775-crash-args-l-e.test.lua b/test/app/gh-4775-crash-args-l-e.test.lua
index 1cccb78a4..7dff8e894 100644
--- a/test/app/gh-4775-crash-args-l-e.test.lua
+++ b/test/app/gh-4775-crash-args-l-e.test.lua
@@ -3,4 +3,6 @@
 --
 child = io.popen('tarantool -e"print(100) os.exit()"')
 child:read()
-child:close()
+-- :close() is omitted, because SIGCHILD may be handled by
+-- libev instead of Lua. In that case :close() with fail with
+-- ECHILD, but it does not matter for this test.