From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp50.i.mail.ru (smtp50.i.mail.ru [94.100.177.110]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 99257469719 for ; Thu, 20 Feb 2020 02:09:10 +0300 (MSK) References: <450bafe2e276ac6e520d4772f82f033873c8039a.1582067172.git.v.shpilevoy@tarantool.org> <20200219085410.GA6469@tarantool.org> From: Vladislav Shpilevoy Message-ID: Date: Thu, 20 Feb 2020 00:09:08 +0100 MIME-Version: 1.0 In-Reply-To: <20200219085410.GA6469@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH 1/1] app: handle concatenated argv name-value correctly List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Munkin Cc: tarantool-patches@dev.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.