From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Ludovic =?UTF-8?Q?Court=C3=A8s?= Newsgroups: gmane.lisp.guile.bugs Subject: bug#52835: [PATCH 0/2] Fix spawning a child not setting standard fds properly Date: Tue, 13 Dec 2022 00:49:06 +0100 Message-ID: <87iligyyh9.fsf@gnu.org> References: <1d64d8e0e292fc3a89bcd491dd8f10171cb7c804.1662358976.git.dev@jpoiret.xyz> <875yex4x9a.fsf_-_@gnu.org> <87sfhl8zml.fsf@jpoiret.xyz> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="3791"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) Cc: 52835@debbugs.gnu.org, Timothy Sample To: Josselin Poiret Original-X-From: bug-guile-bounces+guile-bugs=m.gmane-mx.org@gnu.org Tue Dec 13 00:50:34 2022 Return-path: Envelope-to: guile-bugs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1p4sZ7-0000kG-NU for guile-bugs@m.gmane-mx.org; Tue, 13 Dec 2022 00:50:33 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1p4sYe-0008G6-EC; Mon, 12 Dec 2022 18:50:04 -0500 Original-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 1p4sYd-0008Fi-0t for bug-guile@gnu.org; Mon, 12 Dec 2022 18:50:03 -0500 Original-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 1p4sYc-0004L7-P8 for bug-guile@gnu.org; Mon, 12 Dec 2022 18:50:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1p4sYc-0008Vr-F8 for bug-guile@gnu.org; Mon, 12 Dec 2022 18:50:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Ludovic =?UTF-8?Q?Court=C3=A8s?= Original-Sender: "Debbugs-submit" Resent-CC: bug-guile@gnu.org Resent-Date: Mon, 12 Dec 2022 23:50:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 52835 X-GNU-PR-Package: guile X-GNU-PR-Keywords: patch Original-Received: via spool by 52835-submit@debbugs.gnu.org id=B52835.167088895732693 (code B ref 52835); Mon, 12 Dec 2022 23:50:02 +0000 Original-Received: (at 52835) by debbugs.gnu.org; 12 Dec 2022 23:49:17 +0000 Original-Received: from localhost ([127.0.0.1]:56294 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1p4sXs-0008VF-Sb for submit@debbugs.gnu.org; Mon, 12 Dec 2022 18:49:17 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:57892) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1p4sXr-0008V9-FR for 52835@debbugs.gnu.org; Mon, 12 Dec 2022 18:49:15 -0500 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1p4sXl-0003yL-Rv; Mon, 12 Dec 2022 18:49:09 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-Version:In-Reply-To:Date:References:Subject:To: From; bh=XYwJjWnNjOc2/C4Rpc3acPcTFtAs3Sy1M7X3/5g+hVc=; b=KMj3rmPtteT6KHXDojqC JKCvu35OoJ2tujPnYLcAqP9tuLEuHG5llD66W5wELuhLgrsMCVh/lZ3XkDY7K0b2XGl5ad7qZZXuU oXTxtJBnG2dXsxeqCi/OTyKzLe5GovT65nkU3c5eqfDSL+Lmcglk7Y9qppjmvbcYrdqBKmWDpMRFJ M6nInwGAIkBa7O7BqhE7i9qbEP6wqOoWuZD+aYdI9sOzpheXZbV8tUfDOez3nBpzVfDWChnx5iMRm GR+uAZJONNT6MX2XuspOR/spJ9d5lf9xF62aUQlTbLJejJoT+prabjEs87YuaKsTbH2HE5Nn5Ap4S 7fyp4CaH/ybZYw==; Original-Received: from [82.66.71.168] (helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1p4sXl-0006t3-ET; Mon, 12 Dec 2022 18:49:09 -0500 In-Reply-To: <87sfhl8zml.fsf@jpoiret.xyz> (Josselin Poiret's message of "Sun, 11 Dec 2022 21:16:34 +0100") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-guile@gnu.org List-Id: "Bug reports for GUILE, GNU's Ubiquitous Extension Language" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guile-bounces+guile-bugs=m.gmane-mx.org@gnu.org Original-Sender: bug-guile-bounces+guile-bugs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.lisp.guile.bugs:10472 Archived-At: Hello, Josselin Poiret skribis: >> I would call this one =E2=80=98primitive-spawn=E2=80=99 rather than =E2= =80=98spawn*=E2=80=99 and keep it >> private to (ice-9 popen). > > Is there any reason we would want this to not be accessible to the user? > There are already a bunch of functions that manipulate raw fdes, and > people might want to directly use this to not have to care about ports. Yeah, on second thought I think you=E2=80=99re right: it be can be useful to have it exposed to users. In fact, I think we should provide interfaces that make =E2=80=98primitive-= fork=E2=80=99 unnecessary for most use cases. Exposing that procedure is a step in that direction. >> We could even avoid allocating a port when we=E2=80=99re going to use /d= ev/null >> (and thus go with =E2=80=98open-fdes=E2=80=99 directly), but that=E2=80= =99s a micro-optimization >> we can keep for later. > > Right. I chose to keep the code simple for now, it's too much trouble > having to choose between using ports and fdes. Note that I did a small > benchmark and system* with PATCH v5 is 3x faster than on 3.0.8. vfork > is working wonders. Nice! >>> +++ b/test-suite/tests/posix.test >>> @@ -236,24 +236,24 @@ >>>=20=20 >>> (with-test-prefix "system*" >>>=20=20 >>> - (pass-if "http://bugs.gnu.org/13166" >>> - ;; With Guile up to 2.0.7 included, the child process launched by >>> - ;; `system*' would remain alive after an `execvp' failure. >>> - (let ((me (getpid))) >>> - (and (not (zero? (system* "something-that-does-not-exist"))) >>> - (=3D me (getpid))))) >> >> I=E2=80=99d keep this one, I guess it doesn=E2=80=99t hurt? > > As is, it doesn't work since system* would throw a system exception > because spawn is able to catch that error. Previously, the child would > fail its execvp and die with exit code 127, which system* would return. > >>> - (pass-if-equal "exit code for nonexistent file" >>> - 127 ;aka. EX_NOTFOUND >>> - (status:exit-val (system* "something-that-does-not-exist"))) >> >> It=E2=80=99s good that we have better error reporting thanks to =E2=80= =98posix_spawn=E2=80=99. >> >> However, I don=E2=80=99t think we can change that in 3.0. What about, f= or 3.0, >> adding a layer around =E2=80=98spawn=E2=80=99 so that =E2=80=98system*= =E2=80=99 still returns 127 when >> =E2=80=98spawn=E2=80=99 throws to =E2=80=98system-error=E2=80=99? > > So I've been working on something that would do this, but I noticed that > we have no way to be strictly backwards-compatible: if there's an error > like ENOFILE, we can't get a pid from posix_spawn, and so piped-process > won't have anything to return, whereas before it would return the pid of > the failing child. I'm not sure there's a way to emulate that, unless > we just fork a child that instantly returns 127. Doesn't seem great > though. Right now =E2=80=98system*=E2=80=99 does: pid =3D scm_spawn_process (prog, args, in, out, err); SCM_SYSCALL (wait_result =3D waitpid (scm_to_int (pid), &status, 0)); if (wait_result =3D=3D -1) SCM_SYSERROR; How about introducing decomposing =E2=80=98scm_spawn_process=E2=80=99 so th= at we have a lower-level function we could use (=E2=80=98spawn_process=E2=80=99 below), = roughly like so: ret =3D spawn_process (proc, args, in, out, err, &pid); if (ret !=3D 0) { if (ret =3D=3D ENOMEM) { errno =3D ENOMEM; SCM_SYSERROR; } else /* Emulate old-style failure. TODO: In 3.2, turn that into an exception */ status =3D 127 << 8; } else SCM_SYSCALL (wait_result =3D waitpid (scm_to_int (pid), &status, 0)); Does that make sense? It=E2=80=99s a bit of work to emulate that suboptimal behavior, but I think= it=E2=80=99s important not to change that in 3.0. Thanks for your feedback! Ludo=E2=80=99.