From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp44.i.mail.ru (smtp44.i.mail.ru [94.100.177.104]) (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 038A94696C3 for ; Wed, 15 Apr 2020 07:21:27 +0300 (MSK) Date: Wed, 15 Apr 2020 07:21:23 +0300 From: Alexander Turenko Message-ID: <20200415042123.k5nq2wcdx4xi52mj@tkn_work_nb> References: <20200414131910.GI3072@uranus> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20200414131910.GI3072@uranus> Subject: Re: [Tarantool-patches] [PATCH 10/12] popen: add FIXME re group signal flaw List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cyrill Gorcunov Cc: tarantool-patches@dev.tarantool.org On Tue, Apr 14, 2020 at 04:19:10PM +0300, Cyrill Gorcunov wrote: > On Tue, Apr 14, 2020 at 02:38:19PM +0300, Alexander Turenko wrote: > > It is convenient to have such anchors for known problems. > > > > Part of #4031 > Acked-by: Cyrill Gorcunov > > Actually I need to think about all this more. But I think > it is safe to merge now. I think about it more and also briefly discussed with Cyrill. It seems this problem is a kind of fundamental and we should explain it in the documentation comments rather than mark to fix later. So I rewrote the commit in this way: commit 234b184ffdd9dce234520643f12fec12bbc9fa1a Author: Alexander Turenko Date: Mon Apr 13 13:37:55 2020 +0300 popen: clarify group signaling details Even when ..._SETSID and ..._GROUP_SIGNAL are set, we unable to safely kill a process group after the child process we spawned becomes died. So we don't do that. The behaviour seems to be indefeasible part of Unix process group design. The best that we can do here is describe those details in the documentation comment. NB: It seems that pid namespaces allow to overcome this problem, however it is the Linux specific feature, so we unlikely will use them. Part of #4031 diff --git a/src/lib/core/popen.c b/src/lib/core/popen.c index 411aad03b..5cd7926d1 100644 --- a/src/lib/core/popen.c +++ b/src/lib/core/popen.c @@ -539,6 +539,13 @@ popen_state_str(unsigned int state) * When POPEN_FLAG_GROUP_SIGNAL is set the function sends * a signal to a process group rather than a process. * + * A signal will not be sent if the child process is already + * dead: otherwise we might kill another process that occupies + * the same PID later. This means that if the child process + * dies before its own childs, the function will not send a + * signal to the process group even when ..._SETSID and + * ..._GROUP_SIGNAL are set. + * * Return 0 at success or -1 at failure (and set a diag). * * Possible errors: @@ -597,6 +604,8 @@ popen_send_signal(struct popen_handle *handle, int signo) * - Remove the handle from a living list. * - Free all occupied memory. * + * @see popen_send_signal() for note about ..._GROUP_SIGNAL. + * * Return 0 at success and -1 at failure (and set a diag). * * Possible errors: diff --git a/src/lib/core/popen.h b/src/lib/core/popen.h index 4cdd95175..8cb71e28d 100644 --- a/src/lib/core/popen.h +++ b/src/lib/core/popen.h @@ -99,6 +99,8 @@ enum popen_flag_bits { /* * Send signal to a process group. + * + * @see popen_send_signal() for details. */ POPEN_FLAG_GROUP_SIGNAL_BIT = 16, POPEN_FLAG_GROUP_SIGNAL = (1 << POPEN_FLAG_GROUP_SIGNAL_BIT),