From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Tom Gillespie Newsgroups: gmane.emacs.bugs Subject: bug#56002: src/process.c; make-process fails to clean up stderr process on early exit Date: Tue, 9 Aug 2022 11:59:19 -0700 Message-ID: References: <83pmj9qjgk.fsf@gnu.org> <8735e70xwl.fsf@gnus.org> <831qtrvtg5.fsf@gnu.org> <83czd9ve0n.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="25232"; mail-complaints-to="usenet@ciao.gmane.io" Cc: larsi@gnus.org, 56002@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Tue Aug 09 21:08:26 2022 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1oLUaY-0006Rn-CR for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 09 Aug 2022 21:08:26 +0200 Original-Received: from localhost ([::1]:44508 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oLUaX-0001ug-0A for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 09 Aug 2022 15:08:25 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:40658) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oLUSQ-0002pz-UT for bug-gnu-emacs@gnu.org; Tue, 09 Aug 2022 15:00:04 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:55284) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1oLUSQ-0003Qn-IE for bug-gnu-emacs@gnu.org; Tue, 09 Aug 2022 15:00:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1oLUSQ-0003gT-BH for bug-gnu-emacs@gnu.org; Tue, 09 Aug 2022 15:00:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Tom Gillespie Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 09 Aug 2022 19:00:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 56002 X-GNU-PR-Package: emacs Original-Received: via spool by 56002-submit@debbugs.gnu.org id=B56002.166007157914106 (code B ref 56002); Tue, 09 Aug 2022 19:00:02 +0000 Original-Received: (at 56002) by debbugs.gnu.org; 9 Aug 2022 18:59:39 +0000 Original-Received: from localhost ([127.0.0.1]:45033 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oLUS3-0003fR-6L for submit@debbugs.gnu.org; Tue, 09 Aug 2022 14:59:39 -0400 Original-Received: from mail-pg1-f174.google.com ([209.85.215.174]:43570) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oLUS1-0003fE-Bd for 56002@debbugs.gnu.org; Tue, 09 Aug 2022 14:59:38 -0400 Original-Received: by mail-pg1-f174.google.com with SMTP id h132so12183602pgc.10 for <56002@debbugs.gnu.org>; Tue, 09 Aug 2022 11:59:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc; bh=VlBvticDxxAWa9+GXorAJKq3ymfvJ5VFlNrNOl2RHIg=; b=eWe2fykxdkC/LsH9sLiN5Nd2BC61eHtzvwOq+cIn2wdmZ7Yo2lyt45s7rR4rfZw6s1 b7Yq4EtcO3BMySjBRBFMsgnpxdgkroOdk5k+mUe3S9EGbXG2zCMpVxpU/5xtOG8zdkYF AI26cjJqYuuSJtQOI1MCKCVrVtlEzIC8QMpgY57EzJbBs7WlJlckmZJyj+S4Oy6b6OK1 qD0ANo2YzSPx6+eKlAczl/nVSHdAQQBJMNZfXprFNQeHXbg+dlz8KNensVNYwcARP/WG znQe6+LY3Gn24dJ8OEAIR3pAt356i1ArkESvYv2OmuTvNm61cvH14VfxJbEnBOft7FMh rLdQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc; bh=VlBvticDxxAWa9+GXorAJKq3ymfvJ5VFlNrNOl2RHIg=; b=H+n2kJhM9fKqMNM/Rus59MG6tLoT4VwgLAMkDgCkZ0fA8Uzlx/RY+7Yp1VK6zbYlae hcts8hpiZrpUUY5qfAP91cqeidNP5Et+aXTw/HyA1+BCwZj+sb3fGVHQq6p0HwieU6FF n4YtftKdxbOjAwZmxTGRmLwsA73TV/197lyH+qEr15pOksKHRD+pZOLhT5ihLbjXipkq +z3W6IboVqXVMR0ao/Cj/NRW/T6iqqPICAXU8BLFyJQqlPnhV6WJ/HLPL9XQbB+7X2r0 4Vch0JyV2VzktNyckJCur+w6kRBftcjRXoSKrNG9+59Fsne5UIyixsDEC5KLFzKVX4Rc NxhQ== X-Gm-Message-State: ACgBeo29zsdFOJAESmrFpQVcCrXPfQBA2LtVleWQnuqKK1J7/QFTOKeo 2/K+oMEvT6RTCZgz0cbspI9RvZzHz24IijjoQZ4= X-Google-Smtp-Source: AA6agR7Ebyj/MkQK7ALT0kXk8gafTBXAR6B5+59nGT7qNIhItzRm8Ywi7nOOz9FaCAPLzo+z2bFE8V+eRy46R6OAN8s= X-Received: by 2002:a05:6a00:e85:b0:52b:5db8:f3df with SMTP id bo5-20020a056a000e8500b0052b5db8f3dfmr24188429pfb.14.1660071571181; Tue, 09 Aug 2022 11:59:31 -0700 (PDT) In-Reply-To: <83czd9ve0n.fsf@gnu.org> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:239207 Archived-At: > This is a misunderstanding: I meant "recycled" as in > "garbage-collected". GC in Emacs is supposed to prevent leaks of > memory and resources. You seem to be saying that this somehow doesn't > work in this case. Can you explain why it doesn't work, and which > resources specifically appear to be leaking? Ah. It doesn't work because in this failure mode stderrproc is never gced because it is still running and attached to a buffer. This is because it is in a bad state where it cannot exit because it cannot receive a signal from the non-existent primary process. See the example below where you will be prompted to kill stderr-buffer after sleeping and gc. #+begin_src bash read -r -d '' example <<'EOF' (progn (setq stderr-buffer (generate-new-buffer " rc stderr")) (condition-case nil (make-process :name "process that never actually starts" :stderr stderr-buffer :query-stderr t :command '("i_fail_before_there_can_be_a_return_code")) (error t)) (message "%S" (get-buffer-process stderr-buffer)) (sleep-for 1) (garbage-collect) (message "%S" (get-buffer-process stderr-buffer)) (kill-buffer stderr-buffer)) EOF emacs -Q -batch -eval "${example}" #+end_src > But we have code that errors out in the middle of processing all over > the place, and that is safe in Emacs, because any unused Lisp objects > will be GC'ed soon. Why is this case special? There are a few reasons why this is special. 0. An internally created stderrproc will never be gced if the parent process fails to be created for the reasons listed below. 1. A vfork failure or being passed a command that does not exist causes a make-process to fail in a way that a) leaves stderrproc attached to the stderr buffer and b) leaves stderrproc in a bad state where it cannot receive a signal from the primary process to exit because the primary process never makes it into existence 2. Once created, stderrproc is attached to a buffer and that buffer is not guaranteed to be gced if the caller e.g. passes in a buffer they want to persist. 3. Elisp code can and does interact with stderr buffers that have this zombie stderrproc as their buffer process. > I meant the potential interactions that are not explicitly visible by > reading the code, but instead stem from system-dependent stuff that is > related to how subprocesses are created on different systems. My reading of make-process is that it is impossible for callers in the elisp universe to see an internally created stderrproc until after create-process returns so implicit interactions on the elisp side never happen. On the C side there are already implicit interactions that exist and can fail because the original code as written did not account for them, such as failures during vfork or a command that does not exist, and this patch prevents those. To the best of my knowledge the point to which I moved the call to create stderrproc minimize all possible implicit interactions. It is a pipe process that nothing else cares about or depends on until create_process gets stderrproc's stdout file descriptor and passes it to emacs_spawn as fd_error. > So I'd still be happier if we could deal with the problem without > moving chunks of code around. The alternative is to add code to clean up the stderrproc for any possible failure during make-process after it has been created, though I'm not sure that is actually possible. It is vastly easier, requires much less code, and leaves no doubt as to whether we missed a call that could fail, to simply move the creation of stderrproc to a point after all those potential failure conditions but before it is ever accessed or needed by other parts of the system. To quote my internal notes while debugging the issue: >> there is literally no way to automatically clean up the stderr buffer >> no matter what you do if there is an error during process creation Of course the caller could add a call to get-buffer-process and kill the thing, but there shouldn't even be a process attached to the stderr buffer because the call to make-process failed. Without this patch make-process is fundamentally broken and every caller that wants to clean up the stderr buffer on error but also prompt on exit would have to know about this issue and call get-buffer-process on the stderr buffer and then kill that process manually because it is in a bad state and will not terminate by itself. All this assuming that they could even figure out what was happening and find this bug report.