unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Tom Gillespie <tgbugs@gmail.com>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 56002@debbugs.gnu.org
Subject: bug#56002: src/process.c; make-process fails to clean up stderr process on early exit
Date: Sun, 7 Aug 2022 16:48:44 -0700	[thread overview]
Message-ID: <CA+G3_POV0zB11TFLcYtvHuMSB5Oou8tW1v6K50qbhWrv5f=aPQ@mail.gmail.com> (raw)
In-Reply-To: <CA+G3_PPtJd9L+FWTPBkhKcs0fPAcTqRQzw54oG66fLHrC5mrsQ@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 67 bytes --]

Here is an update to the 0002 patch to account for recent changes.

[-- Attachment #2: 0002-make-process-stderrproc-cleanup-on-failure-decouple-v2.patch --]
[-- Type: text/x-patch, Size: 4429 bytes --]

From f8830045187d28c90ee6167eafc79a0019c41ab7 Mon Sep 17 00:00:00 2001
From: Tom Gillespie <tgbugs@gmail.com>
Date: Sun, 7 Aug 2022 16:43:13 -0700
Subject: [PATCH] make-process stderrproc cleanup on failure decouple stderr
 query

* src/process.c (Fmake_process): Move the call to create the stderr
process as late as possible to avoid having to clean up stderrproc in
the event of an error prior to the call to create_process. This change
is needed to ensure that when called with :query-stderr t (aka the
default behavior prior to the addition of :query-stderr) make-process
will not leak the stderr process if a call to make-process fails.
Also adds a new keyword argument :query-stderr to control whether to
query on exit the stderr process (if one is created). (bug#56002)

Now with changes to account for intervening changes to pty code.
---
 src/process.c | 45 ++++++++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 17 deletions(-)

diff --git a/src/process.c b/src/process.c
index 23479c0619..11f85c5d1d 100644
--- a/src/process.c
+++ b/src/process.c
@@ -1762,6 +1762,11 @@ DEFUN ("make-process", Fmake_process, Smake_process, 0, MANY, 0,
 :noquery BOOL -- When exiting Emacs, query the user if BOOL is nil and
 the process is running.  If BOOL is not given, query before exiting.
 
+:query-stderr BOOL -- When exiting Emacs, query the user if BOOL is
+non-nil and the stderr process is running. If BOOL is not given, do
+not query before exiting. This has no effect if the value passed to
+:stderr is a process.
+
 :stop BOOL -- BOOL must be nil.  The `:stop' key is ignored otherwise
 and is retained for compatibility with other process types such as
 pipe processes.  Asynchronous subprocesses never start in the
@@ -1837,8 +1842,6 @@ DEFUN ("make-process", Fmake_process, Smake_process, 0, MANY, 0,
   if (!NILP (program))
     CHECK_STRING (program);
 
-  bool query_on_exit = NILP (plist_get (contact, QCnoquery));
-
   stderrproc = Qnil;
   xstderr = plist_get (contact, QCstderr);
   if (PROCESSP (xstderr))
@@ -1848,16 +1851,7 @@ DEFUN ("make-process", Fmake_process, Smake_process, 0, MANY, 0,
       stderrproc = xstderr;
     }
   else if (!NILP (xstderr))
-    {
-      CHECK_STRING (program);
-      stderrproc = CALLN (Fmake_pipe_process,
-			  QCname,
-			  concat2 (name, build_string (" stderr")),
-			  QCbuffer,
-			  Fget_buffer_create (xstderr, Qnil),
-			  QCnoquery,
-			  query_on_exit ? Qnil : Qt);
-    }
+    CHECK_STRING (program);
 
   proc = make_process (name);
   record_unwind_protect (start_process_unwind, proc);
@@ -1870,7 +1864,7 @@ DEFUN ("make-process", Fmake_process, Smake_process, 0, MANY, 0,
   pset_filter (XPROCESS (proc), plist_get (contact, QCfilter));
   pset_command (XPROCESS (proc), Fcopy_sequence (command));
 
-  if (!query_on_exit)
+  if (!NILP (plist_get (contact, QCnoquery)))
     XPROCESS (proc)->kill_without_query = 1;
   tem = plist_get (contact, QCstop);
   /* Normal processes can't be started in a stopped state, see
@@ -1889,9 +1883,6 @@ DEFUN ("make-process", Fmake_process, Smake_process, 0, MANY, 0,
 	is_pty_from_symbol (tem);
     }
 
-  if (!NILP (stderrproc))
-    pset_stderrproc (XPROCESS (proc), stderrproc);
-
 #ifdef HAVE_GNUTLS
   /* AKA GNUTLS_INITSTAGE(proc).  */
   verify (GNUTLS_STAGE_EMPTY == 0);
@@ -2057,10 +2048,29 @@ DEFUN ("make-process", Fmake_process, Smake_process, 0, MANY, 0,
 	  tem = XCDR (tem);
 	}
 
+      if (!PROCESSP (xstderr) && !NILP (xstderr))
+	{
+	  stderrproc = CALLN (Fmake_pipe_process,
+			      QCname,
+			      concat2 (name, build_string (" stderr")),
+			      QCbuffer,
+			      Fget_buffer_create (xstderr, Qnil),
+			      QCnoquery,
+			      NILP (plist_get (contact, QCquery_stderr)) ? Qt : Qnil);
+	}
+
+      if (!NILP (stderrproc))
+	pset_stderrproc (XPROCESS (proc), stderrproc);
+
       create_process (proc, new_argv, current_dir);
     }
   else
-    create_pty (proc);
+    {
+      if (!NILP (stderrproc))
+	pset_stderrproc (XPROCESS (proc), stderrproc);
+
+      create_pty (proc);
+    }
 
   return SAFE_FREE_UNBIND_TO (count, proc);
 }
@@ -8609,6 +8619,7 @@ syms_of_process (void)
   DEFSYM (Qnsm_verify_connection, "nsm-verify-connection");
   DEFSYM (QClog, ":log");
   DEFSYM (QCnoquery, ":noquery");
+  DEFSYM (QCquery_stderr, ":query-stderr");
   DEFSYM (QCstop, ":stop");
   DEFSYM (QCplist, ":plist");
   DEFSYM (QCcommand, ":command");
-- 
2.35.1


  reply	other threads:[~2022-08-07 23:48 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-15 22:38 bug#56002: src/process.c; make-process fails to clean up stderr process on early exit Tom Gillespie
2022-06-16  2:28 ` bug#56002: update with an additional example Tom Gillespie
2022-06-16  5:13 ` bug#56002: src/process.c; make-process fails to clean up stderr process on early exit Eli Zaretskii
2022-06-16  6:11   ` Tom Gillespie
2022-06-29 21:17     ` Tom Gillespie
2022-08-07 23:48       ` Tom Gillespie [this message]
2022-08-08 11:36         ` Lars Ingebrigtsen
2022-08-08 11:57           ` Eli Zaretskii
2022-08-08 18:54             ` Tom Gillespie
2022-08-09 11:43               ` Eli Zaretskii
2022-08-09 18:59                 ` Tom Gillespie
2022-08-10 18:06                   ` Eli Zaretskii
2022-08-11  2:33                     ` Tom Gillespie
2022-08-11  6:30                       ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CA+G3_POV0zB11TFLcYtvHuMSB5Oou8tW1v6K50qbhWrv5f=aPQ@mail.gmail.com' \
    --to=tgbugs@gmail.com \
    --cc=56002@debbugs.gnu.org \
    --cc=eliz@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).