From debbugs-submit-bounces@debbugs.gnu.org Mon May 25 08:31:43 2020 Received: (at 41507) by debbugs.gnu.org; 25 May 2020 12:31:43 +0000 Received: from localhost ([127.0.0.1]:40122 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jdCGc-0006r1-TO for submit@debbugs.gnu.org; Mon, 25 May 2020 08:31:43 -0400 Received: from eggs.gnu.org ([209.51.188.92]:57838) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jdCGa-0006qn-FI for 41507@debbugs.gnu.org; Mon, 25 May 2020 08:31:41 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:48290) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jdCGU-0004Hy-G5; Mon, 25 May 2020 08:31:34 -0400 Received: from lfbn-ann-1-136-86.w86-200.abo.wanadoo.fr ([86.200.104.86]:39508 helo=meru) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1jdCGT-00016i-PU; Mon, 25 May 2020 08:31:34 -0400 From: Mathieu Othacehe To: Ludovic =?utf-8?Q?Court=C3=A8s?= Subject: Re: [bug#41507] [PATCH Shepherd 2/2] shepherd: Use 'signalfd' when possible. References: <20200524143700.6378-1-ludo@gnu.org> <20200524143700.6378-2-ludo@gnu.org> Date: Mon, 25 May 2020 14:31:30 +0200 In-Reply-To: <20200524143700.6378-2-ludo@gnu.org> ("Ludovic \=\?utf-8\?Q\?Cour\?\= \=\?utf-8\?Q\?t\=C3\=A8s\=22's\?\= message of "Sun, 24 May 2020 16:37:00 +0200") Message-ID: <87a71ww5zx.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 41507 Cc: 41507@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) Hello Ludo, This is a big improvement, thank you! > + (lambda args > + (if (= ENOSYS (system-error-errno args)) > + #f > + (apply throw args))))) Maybe: (and (= ENOSYS (system-error-errno args)) (apply throw args)) > + > +(define (handle-signal-port port) > + "Read from PORT, a signalfd port, and handle the signal accordingly." > + (let ((signal (consume-signalfd-siginfo port))) > + (cond ((= signal SIGCHLD) > + (handle-SIGCHLD)) > + ((= signal SIGINT) > + (catch 'quit > + (lambda () > + (stop root-service)) > + quit-exception-handler)) Maybe we should create a handle-SIGINT, to make sure that the same code is shared with the sigaction handler? > + (begin > + ;; Unblock any signals that might have been blocked by the parent > + ;; process. > + (unblock-signals (list SIGCHLD SIGINT SIGHUP SIGTERM)) This made me realize that we may want to disable/reset SIGINT and SIGHUP, in the same way that we do for SIGTERM? This is not related to your patch anyway. You could also extend the comment to say that it is only necessary if using the signalfd mechanism (because signals are not blocked otherwise). In any case, this looks fine! Thanks, Mathieu