Subject: Re: bug#56002: src/process.c; make-process fails to clean up stderr process on early exit

From: Tom Gillespie <[email-masked]>
Date: Sun, 7 Aug 2022 16:48:44 -0700

Here is an update to the 0002 patch to account for recent changes.
From f8830045187d28c90ee6167eafc79a0019c41ab7 Mon Sep 17 00:00:00 2001
From: Tom Gillespie <[email-masked]>
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