From patchwork Sat Feb 25 22:08:01 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: ulfvonbelow X-Patchwork-Id: 10380 Return-Path: X-Original-To: patchwork@mira.cbaines.net Delivered-To: patchwork@mira.cbaines.net Received: by mira.cbaines.net (Postfix, from userid 113) id 4AC4616BBF; Sun, 26 Feb 2023 13:49:01 +0000 (GMT) X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on mira.cbaines.net X-Spam-Level: X-Spam-Status: No, score=-1.8 required=5.0 tests=DKIM_INVALID,DKIM_SIGNED, MAILING_LIST_MULTI,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mira.cbaines.net (Postfix) with ESMTPS id 230EE168F8 for ; Sun, 26 Feb 2023 13:48:58 +0000 (GMT) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pWHOS-0007Ic-1B; Sun, 26 Feb 2023 08:48:48 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pWCRu-0004Ii-RI for guix-patches@gnu.org; Sun, 26 Feb 2023 03:32:03 -0500 Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pWCRu-00043M-HX for guix-patches@gnu.org; Sun, 26 Feb 2023 03:32:02 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1pWCRu-0005MO-DJ for guix-patches@gnu.org; Sun, 26 Feb 2023 03:32:02 -0500 X-Loop: help-debbugs@gnu.org Subject: [bug#61803] [PATCH 0/3] [shepherd] improve race-free spawn+wait Resent-From: Ulf Herrman Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Sun, 26 Feb 2023 08:32:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 61803 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: 61803@debbugs.gnu.org X-Debbugs-Original-To: guix-patches@gnu.org Received: via spool by submit@debbugs.gnu.org id=B.167740028220549 (code B ref -1); Sun, 26 Feb 2023 08:32:02 +0000 Received: (at submit) by debbugs.gnu.org; 26 Feb 2023 08:31:22 +0000 Received: from localhost ([127.0.0.1]:42287 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pWCRD-0005LJ-BK for submit@debbugs.gnu.org; Sun, 26 Feb 2023 03:31:22 -0500 Received: from lists.gnu.org ([209.51.188.17]:49856) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pW2iF-0006yJ-Ld for submit@debbugs.gnu.org; Sat, 25 Feb 2023 17:08:16 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pW2iF-0003ji-DN for guix-patches@gnu.org; Sat, 25 Feb 2023 17:08:15 -0500 Received: from tilde.club ([2607:5300:204:4340::114]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pW2iD-0005B2-8c for guix-patches@gnu.org; Sat, 25 Feb 2023 17:08:15 -0500 Received: by tilde.club (Postfix, from userid 5378) id DA07B220B360C; Sat, 25 Feb 2023 22:08:07 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 tilde.club DA07B220B360C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tilde.club; s=mail; t=1677362887; bh=d/Jb/ZYM6YsbWvuZyUZ8Uxm81yEvsmppozkuXCRm9wU=; h=From:To:Subject:Date:From; b=akrLCwLuKpjHpbiZVZIa2GX7CyNlO3Q/ey78T85zSVKCVg0ZXn2QH67mB7eyTOH7+ /AENJkJv7rg4Z4ltlL06eL/xlpOW6R/so2SlPYgCmXP8Dio7J4upX0RlyFZz52omRQ NK3fY8Alc3oU6tpOLrRr+yPcJGA0eSr+TNMlv33w= From: Ulf Herrman User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) Date: Sat, 25 Feb 2023 16:08:01 -0600 Message-ID: <87cz5x1jr2.fsf@tilde.club> MIME-Version: 1.0 Received-SPF: pass client-ip=2607:5300:204:4340::114; envelope-from=striness@tilde.club; helo=tilde.club X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-Mailman-Approved-At: Sun, 26 Feb 2023 03:31:18 -0500 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-Mailman-Approved-At: Sun, 26 Feb 2023 08:48:46 -0500 X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+patchwork=mira.cbaines.net@gnu.org Sender: guix-patches-bounces+patchwork=mira.cbaines.net@gnu.org X-getmail-retrieved-from-mailbox: Patches These patches fill out shepherd's procedures for running processes to completion. They add a replacement for 'system' to complement the existing replacement for 'system*', and add a 'fork+exec+wait-process' procedure so that the flexibility of that family of procedures is available for this use case as well. It also improves error handling in the event that an exception occurs while spawning a process in the process monitor, which would normally kill that essential fiber. Note: I previously tried to send this to guix-devel, but it didn't seem to make it (I didn't see it in the archives after half a day), and after some consideration I recalled that guix-patches exists. Is this the right place for shepherd patches? From 177592ee9d4b7fc6dcc80e545e8ad615a1d6786c Mon Sep 17 00:00:00 2001 From: ulfvonbelow Date: Sat, 25 Feb 2023 00:56:57 -0600 Subject: [PATCH 3/3] service: add spawn-shell-command replacement for `system'. We already have a replacement for `system*' that avoids racing, but not for `system'. * configure.ac (SHELL): new substitution variable. * modules/shepherd/system.scm.in (%shell-filename): new variable. * modules/shepherd/service.scm (spawn-shell-command, real-system): new procedures. * modules/shepherd.scm (main): replace `system' with `spawn-shell-command'. --- configure.ac | 1 + modules/shepherd.scm | 7 +++++-- modules/shepherd/service.scm | 13 +++++++++++++ modules/shepherd/system.scm.in | 5 ++++- 4 files changed, 23 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index 6f681dc..19c177a 100644 --- a/configure.ac +++ b/configure.ac @@ -32,6 +32,7 @@ guilemoduledir="${datarootdir}/guile/site/$GUILE_EFFECTIVE_VERSION" guileobjectdir="${libdir}/guile/$GUILE_EFFECTIVE_VERSION/site-ccache" AC_SUBST([guilemoduledir]) AC_SUBST([guileobjectdir]) +AC_SUBST([SHELL]) dnl Check for extra dependencies. GUILE_MODULE_AVAILABLE([have_fibers], [(fibers)]) diff --git a/modules/shepherd.scm b/modules/shepherd.scm index cce0507..1f6342e 100644 --- a/modules/shepherd.scm +++ b/modules/shepherd.scm @@ -420,8 +420,10 @@ already ~a threads running, disabling 'signalfd' support") ;; Replace the default 'system*' binding with one that ;; cooperates instead of blocking on 'waitpid'. - (let ((real-system* system*)) + (let ((real-system* system*) + (real-system system)) (set! system* spawn-command) + (set! system spawn-shell-command) ;; Restore 'system*' after fork. (set! primitive-fork @@ -430,7 +432,8 @@ already ~a threads running, disabling 'signalfd' support") (let ((result (real-fork))) (when (zero? result) (set! primitive-fork real-fork) - (set! system* real-system*)) + (set! system* real-system*) + (set! system real-system)) result))))) (run-daemon #:socket-file socket-file diff --git a/modules/shepherd/service.scm b/modules/shepherd/service.scm index a36e486..f8df3a9 100644 --- a/modules/shepherd/service.scm +++ b/modules/shepherd/service.scm @@ -81,6 +81,7 @@ handle-SIGCHLD with-process-monitor spawn-command + spawn-shell-command %precious-signals register-services provided-by @@ -1938,6 +1939,18 @@ context. The process monitoring fiber is responsible for handling (spawn-via-monitor (list (cons program arguments))) (apply system* program arguments))) +(define real-system system) + +(define* (spawn-shell-command #:optional command) + "Like 'system' but do not block while waiting for COMMAND to terminate." + (if (current-process-monitor) + (if command + (spawn-command %shell-filename "-c" command) + #t) + (if command + (real-system command) + (real-system)))) + (define (fork+exec+wait-command command . arguments) "Like 'fork+exec' but also wait for PROGRAM to terminate, giving its exit status." diff --git a/modules/shepherd/system.scm.in b/modules/shepherd/system.scm.in index 29357aa..4646e81 100644 --- a/modules/shepherd/system.scm.in +++ b/modules/shepherd/system.scm.in @@ -41,7 +41,8 @@ unblock-signals set-blocked-signals with-blocked-signals - without-automatic-finalization)) + without-automatic-finalization + %shell-filename)) ;; The constants. (define RB_AUTOBOOT @RB_AUTOBOOT@) @@ -328,3 +329,5 @@ Turning finalization off shuts down the finalization thread as a side effect." exp ...) (lambda () (%set-automatic-finalization-enabled?! enabled?))))) + +(define %shell-filename "@SHELL@") -- 2.38.1