bug#33154: 27.0.50; create_process on Darwin should not invoke setsid() after vfork() [PATCH]

bug#33154: 27.0.50; create_process on Darwin should not invoke setsid() after vfork() [PATCH]

[Filipp Gunbin]

This resulted from analysis of bug 33050.  To avoid repetition, here's
the message with explanation:
http://lists.gnu.org/archive/html/bug-gnu-emacs/2018-10/msg00763.html

Suggested patch is below.

Thanks.

In GNU Emacs 27.0.50 (build 12, x86_64-apple-darwin17.7.0, NS appkit-1561.60 Version 10.13.6 (Build 17G65))
 of 2018-10-25 built on fgunbin.playteam.ru
Repository revision: f1f1687fcd8d48cd519c0f2977bcecbf394a7f01
System Description:  Mac OS X 10.13.6


diff --git a/src/process.c b/src/process.c
index 6cda4f27ac..1f8810927d 100644
--- a/src/process.c
+++ b/src/process.c
@@ -2066,21 +2066,7 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
   int volatile forkerr_volatile = forkerr;
   struct Lisp_Process *p_volatile = p;
 
-#ifdef DARWIN_OS
-  /* Darwin doesn't let us run setsid after a vfork, so use fork when
-     necessary.  Also, reset SIGCHLD handling after a vfork, as
-     apparently macOS can mistakenly deliver SIGCHLD to the child.  */
-  if (pty_flag)
-    pid = fork ();
-  else
-    {
-      pid = vfork ();
-      if (pid == 0)
-	signal (SIGCHLD, SIG_DFL);
-    }
-#else
   pid = vfork ();
-#endif
 
   current_dir = current_dir_volatile;
   lisp_pty_name = lisp_pty_name_volatile;
@@ -2091,15 +2077,35 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
   p = p_volatile;
 
   pty_flag = p->pty_flag;
-
   if (pid == 0)
 #endif /* not WINDOWSNT */
     {
+#ifdef DARWIN_OS
+      /* Work around a macOS bug, where SIGCHLD is apparently
+	 delivered to a vforked child instead of to its parent.  See:
+	 https://lists.gnu.org/r/emacs-devel/2017-05/msg00342.html
+      */
+      signal (SIGCHLD, SIG_DFL);
+#endif
+
       /* Make the pty be the controlling terminal of the process.  */
 #ifdef HAVE_PTYS
       /* First, disconnect its current controlling terminal.
 	 Do this even if !PTY_FLAG; see Bug#30762.  */
+#ifdef DARWIN_OS
+      /* Darwin doesn't let us run setsid after a vfork, so use
+         TIOCNOTTY when necessary. */
+      {
+	int j = emacs_open (DEV_TTY, O_RDWR, 0);
+	if (j >= 0)
+	  {
+	    ioctl (j, TIOCNOTTY, 0);
+	    emacs_close (j);
+	  }
+      }
+#else
       setsid ();
+#endif
       /* Make the pty's terminal the controlling terminal.  */
       if (pty_flag && forkin >= 0)
 	{

bug#33154: 27.0.50; create_process on Darwin should not invoke setsid() after vfork() [PATCH]

[Alan Third]

On Thu, Oct 25, 2018 at 10:30:12PM +0300, Filipp Gunbin wrote:
> This resulted from analysis of bug 33050.  To avoid repetition, here's
> the message with explanation:
> http://lists.gnu.org/archive/html/bug-gnu-emacs/2018-10/msg00763.html

Sorry, I didn’t notice the other emails before now.

This looks fine to me, but I’m no expert on this stuff.

Just one point, though:

>        /* Make the pty be the controlling terminal of the process.  */
>  #ifdef HAVE_PTYS
>        /* First, disconnect its current controlling terminal.
>  	 Do this even if !PTY_FLAG; see Bug#30762.  */
> +#ifdef DARWIN_OS
> +      /* Darwin doesn't let us run setsid after a vfork, so use
> +         TIOCNOTTY when necessary. */
> +      {
> +	int j = emacs_open (DEV_TTY, O_RDWR, 0);
> +	if (j >= 0)
> +	  {
> +	    ioctl (j, TIOCNOTTY, 0);
> +	    emacs_close (j);
> +	  }
> +      }
> +#else
>        setsid ();
> +#endif

The main block of this code looks identical to the BSD specific code
just underneath (#ifdef TIOCNOTTY), is there a reason we can’t reuse
that?

-- 
Alan Third

bug#33154: 27.0.50; create_process on Darwin should not invoke setsid() after vfork() [PATCH]

[Filipp Gunbin]

Well, my initial patch fixed the pipe case, but broke the pty case.
That became visible after make bootstrap.

This new patch seems to work.  It attempts to handle all cases:

- When not on Darwin, just do vfork() and setsid()

- When on Darwin and pipe mode, do the same as call_process(): vfork(),
  then detach from the current controlling terminal via TIOCNOTTY

- When on Darwin and pty mode, do fork() and later setsid().  We should
  create new session to be able to use just allocated tty.

I'll use it for some time, then report the result.

Filipp


diff --git a/src/process.c b/src/process.c
index 6cda4f27ac..9125936eb9 100644
--- a/src/process.c
+++ b/src/process.c
@@ -2099,7 +2099,23 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
 #ifdef HAVE_PTYS
       /* First, disconnect its current controlling terminal.
 	 Do this even if !PTY_FLAG; see Bug#30762.  */
+#ifdef DARWIN_OS
+      /* Darwin doesn't let us run setsid after a vfork, so use
+	 TIOCNOTTY when necessary. */
+      if (pty_flag)
+	setsid ();
+      else
+	{
+	  int j = emacs_open (DEV_TTY, O_RDWR, 0);
+	  if (j >= 0)
+	    {
+	      ioctl (j, TIOCNOTTY, 0);
+	      emacs_close (j);
+	    }
+	}
+#else
       setsid ();
+#endif
       /* Make the pty's terminal the controlling terminal.  */
       if (pty_flag && forkin >= 0)
 	{

bug#33154: 27.0.50; create_process on Darwin should not invoke setsid() after vfork() [PATCH]

[Paul Eggert]

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

> The main block of this code looks identical to the BSD specific code
> just underneath (#ifdef TIOCNOTTY), is there a reason we can’t reuse
> that?

I don't see why not. Proposed patch attached; please give it a try as I 
don't use macOS.


[-- Attachment #2: 0001-Let-macOS-vfork-to-create-processes.patch --]
[-- Type: text/x-patch, Size: 4209 bytes --]

From c72e828018b68ed1ce1cad9205b6037f86112a28 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 5 Nov 2018 09:25:46 -0800
Subject: [PATCH] Let macOS vfork to create processes

* src/process.c (create_process) [DARWIN_OS]:
Use vfork, not fork.  However, do not use setsid;
instead, rely on TIOCSPGRP.  Move SIGCHLD ignoring
to its logical place next to the ignoring of other signals.
---
 src/process.c | 51 +++++++++++++++++++++++----------------------------
 1 file changed, 23 insertions(+), 28 deletions(-)

diff --git a/src/process.c b/src/process.c
index 6cda4f27ac..dd8beed117 100644
--- a/src/process.c
+++ b/src/process.c
@@ -2066,21 +2066,7 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
   int volatile forkerr_volatile = forkerr;
   struct Lisp_Process *p_volatile = p;
 
-#ifdef DARWIN_OS
-  /* Darwin doesn't let us run setsid after a vfork, so use fork when
-     necessary.  Also, reset SIGCHLD handling after a vfork, as
-     apparently macOS can mistakenly deliver SIGCHLD to the child.  */
-  if (pty_flag)
-    pid = fork ();
-  else
-    {
-      pid = vfork ();
-      if (pid == 0)
-	signal (SIGCHLD, SIG_DFL);
-    }
-#else
   pid = vfork ();
-#endif
 
   current_dir = current_dir_volatile;
   lisp_pty_name = lisp_pty_name_volatile;
@@ -2098,18 +2084,22 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
       /* Make the pty be the controlling terminal of the process.  */
 #ifdef HAVE_PTYS
       /* First, disconnect its current controlling terminal.
-	 Do this even if !PTY_FLAG; see Bug#30762.  */
+	 Do this even if !PTY_FLAG; see Bug#30762.
+	 However, Darwin should not invoke setsid after vfork (see
+	 Bug#33050 and Bug#33154), so rely TIOCNOTTY below instead.  */
+# ifndef DARWIN_OS
       setsid ();
+# endif
       /* Make the pty's terminal the controlling terminal.  */
       if (pty_flag && forkin >= 0)
 	{
-#ifdef TIOCSCTTY
+# ifdef TIOCSCTTY
 	  /* We ignore the return value
 	     because faith@cs.unc.edu says that is necessary on Linux.  */
 	  ioctl (forkin, TIOCSCTTY, 0);
-#endif
+# endif
 	}
-#if defined (LDISC1)
+# ifdef LDISC1
       if (pty_flag && forkin >= 0)
 	{
 	  struct termios t;
@@ -2118,17 +2108,15 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
 	  if (tcsetattr (forkin, TCSANOW, &t) < 0)
 	    emacs_perror ("create_process/tcsetattr LDISC1");
 	}
-#else
-#if defined (NTTYDISC) && defined (TIOCSETD)
+# elif defined NTTYDISC && defined TIOCSETD
       if (pty_flag && forkin >= 0)
 	{
 	  /* Use new line discipline.  */
 	  int ldisc = NTTYDISC;
 	  ioctl (forkin, TIOCSETD, &ldisc);
 	}
-#endif
-#endif
-#ifdef TIOCNOTTY
+# endif
+# ifdef TIOCNOTTY
       /* In 4.3BSD, the TIOCSPGRP bug has been fixed, and now you
 	 can do TIOCSPGRP only to the process's controlling tty.  */
       if (pty_flag)
@@ -2142,9 +2130,9 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
 	      emacs_close (j);
 	    }
 	}
-#endif /* TIOCNOTTY */
+# endif
 
-#if !defined (DONT_REOPEN_PTY)
+# ifndef DONT_REOPEN_PTY
 /*** There is a suggestion that this ought to be a
      conditional on TIOCSPGRP, or !defined TIOCSCTTY.
      Trying the latter gave the wrong results on Debian GNU/Linux 1.1;
@@ -2168,14 +2156,14 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
 	    }
 
 	}
-#endif /* not DONT_REOPEN_PTY */
+# endif
 
-#ifdef SETUP_SLAVE_PTY
+# ifdef SETUP_SLAVE_PTY
       if (pty_flag)
 	{
 	  SETUP_SLAVE_PTY;
 	}
-#endif /* SETUP_SLAVE_PTY */
+# endif
 #endif /* HAVE_PTYS */
 
       signal (SIGINT, SIG_DFL);
@@ -2187,6 +2175,13 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
       /* Emacs ignores SIGPIPE, but the child should not.  */
       signal (SIGPIPE, SIG_DFL);
 
+#ifdef DARWIN_OS
+      /* https://lists.gnu.org/r/emacs-devel/2017-05/msg00342.html
+	 says macOS has a bug where SIGCHLD is delivered to a vforked
+	 child instead of to its parent.  */
+      signal (SIGCHLD, SIG_DFL);
+#endif
+
       /* Stop blocking SIGCHLD in the child.  */
       unblock_child_signal (&oldset);
 
-- 
2.19.1

bug#33154: 27.0.50; create_process on Darwin should not invoke setsid() after vfork() [PATCH]

[Filipp Gunbin]

On 05/11/2018 09:28 -0800, Paul Eggert wrote:

>> The main block of this code looks identical to the BSD specific code
>> just underneath (#ifdef TIOCNOTTY), is there a reason we can’t reuse
>> that?
>
> I don't see why not. Proposed patch attached; please give it a try as
> I don't use macOS.

Paul, thanks for the patch.

I'm seeing some problems with my fix - specifically, with sudo run as
async shell command:

"sudo: no tty present and no askpass program specified"

and with tramp when trying to connect to anything via ssh:

Permission denied, please try again.
Permission denied, please try again.
Received disconnect from <my-ip> port 22:2: Too many authentication failures
Disconnected from <my-ip> port 22

I didn't have time to look into it.  When I handle it, I'll try your
patch also and will write back.

Filipp

bug#33154: 27.0.50; create_process on Darwin should not invoke setsid() after vfork() [PATCH]

[Filipp Gunbin]

Paul, I think this won't work:

man tty (4):
     TIOCSCTTY void
                 Make the terminal the controlling terminal for the process
                 (the process must not currently have a controlling terminal).

In your patch, we don't detach from current (Emacs's) controlling
terminal before doing TIOCSCTTY.

I think what Alan meant as "reuse" was to extract the code into some
function.

Filipp

bug#33154: 27.0.50; create_process on Darwin should not invoke setsid() after vfork() [PATCH]

[Paul Eggert]

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

Filipp Gunbin wrote:
> In your patch, we don't detach from current (Emacs's) controlling
> terminal before doing TIOCSCTTY.

Ah, OK. I see also that vfork won't work on Darwin if pty mode is used, since 
Emacs wants to create a new session and Darwin setsid always fails in a vforked 
child that has not yet execed.

However, your patch introduces another duplicate of the open/TIOCNOTTY/close 
fallback code, making three duplicates in all. How about if we coalesce these 
duplicates into a function and then call that function? Also, I think we can 
call the function from just two places (not three). Furthermore, I think it'd be 
more robust if Emacs does setsid everywhere (with a fallback to 
open/TIOCNOTTY/close everywhere TIOCNOTTY is available), not just Darwin. 
Proposed patch (against master) attached.

[-- Attachment #2: 0001-Dissociate-controlling-tty-better-on-Darwin.patch --]
[-- Type: text/x-patch, Size: 3706 bytes --]

From cff3581f79d5eeb251fb683250bf48da3f68895a Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Tue, 6 Nov 2018 23:30:26 -0800
Subject: [PATCH] Dissociate controlling tty better on Darwin

* src/process.c (dissociate_controlling_tty): New function.
(create_process): Use it to dissociate controlling tty if setsid
fails, which happens on Darwin after a vfork (Bug#33154).
Do this on all platforms, not just on Darwin, as a similar
problem is plausible elsewhere.
* src/callproc.c (call_process): Use the new function here, too,
for consistency and to avoid duplicate code.
---
 src/callproc.c | 14 +-------------
 src/process.c  | 40 ++++++++++++++++++++++------------------
 src/process.h  |  1 +
 3 files changed, 24 insertions(+), 31 deletions(-)

diff --git a/src/callproc.c b/src/callproc.c
index a2cfd2e94d..9f47c79b81 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -643,19 +643,7 @@ call_process (ptrdiff_t nargs, Lisp_Object *args, int filefd,
 #endif
 
       unblock_child_signal (&oldset);
-
-#ifdef DARWIN_OS
-      /* Darwin doesn't let us run setsid after a vfork, so use
-         TIOCNOTTY when necessary. */
-      int j = emacs_open (DEV_TTY, O_RDWR, 0);
-      if (j >= 0)
-        {
-          ioctl (j, TIOCNOTTY, 0);
-          emacs_close (j);
-        }
-#else
-      setsid ();
-#endif
+      dissociate_controlling_tty ();
 
       /* Emacs ignores SIGPIPE, but the child should not.  */
       signal (SIGPIPE, SIG_DFL);
diff --git a/src/process.c b/src/process.c
index 6cda4f27ac..7e78e172d3 100644
--- a/src/process.c
+++ b/src/process.c
@@ -1949,6 +1949,26 @@ close_process_fd (int *fd_addr)
     }
 }
 
+void
+dissociate_controlling_tty (void)
+{
+  if (setsid () < 0)
+    {
+#ifdef TIOCNOTTY
+      /* Needed on Darwin after vfork, since setsid fails in a vforked
+	 child that has not execed.
+	 I wonder: would just ioctl (fd, TIOCNOTTY, 0) work here, for
+	 some fd that the caller already has?  */
+      int ttyfd = emacs_open (DEV_TTY, O_RDWR, 0);
+      if (0 <= ttyfd)
+	{
+	  ioctl (ttyfd, TIOCNOTTY, 0);
+	  emacs_close (ttyfd);
+	}
+#endif
+    }
+}
+
 /* Indexes of file descriptors in open_fds.  */
 enum
   {
@@ -2097,9 +2117,8 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
     {
       /* Make the pty be the controlling terminal of the process.  */
 #ifdef HAVE_PTYS
-      /* First, disconnect its current controlling terminal.
-	 Do this even if !PTY_FLAG; see Bug#30762.  */
-      setsid ();
+      dissociate_controlling_tty ();
+
       /* Make the pty's terminal the controlling terminal.  */
       if (pty_flag && forkin >= 0)
 	{
@@ -2128,21 +2147,6 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
 	}
 #endif
 #endif
-#ifdef TIOCNOTTY
-      /* In 4.3BSD, the TIOCSPGRP bug has been fixed, and now you
-	 can do TIOCSPGRP only to the process's controlling tty.  */
-      if (pty_flag)
-	{
-	  /* I wonder: would just ioctl (0, TIOCNOTTY, 0) work here?
-	     I can't test it since I don't have 4.3.  */
-	  int j = emacs_open (DEV_TTY, O_RDWR, 0);
-	  if (j >= 0)
-	    {
-	      ioctl (j, TIOCNOTTY, 0);
-	      emacs_close (j);
-	    }
-	}
-#endif /* TIOCNOTTY */
 
 #if !defined (DONT_REOPEN_PTY)
 /*** There is a suggestion that this ought to be a
diff --git a/src/process.h b/src/process.h
index 3c6dd7b91f..67b783400d 100644
--- a/src/process.h
+++ b/src/process.h
@@ -300,6 +300,7 @@ extern Lisp_Object network_interface_info (Lisp_Object);
 extern Lisp_Object remove_slash_colon (Lisp_Object);
 
 extern void update_processes_for_thread_death (Lisp_Object);
+extern void dissociate_controlling_tty (void);
 
 INLINE_HEADER_END
 
-- 
2.17.1

bug#33154: 27.0.50; create_process on Darwin should not invoke setsid() after vfork() [PATCH]

[Filipp Gunbin]

Yes, dissociate_controlling_tty looks nice.  I thought about checking
what setsid() returns too, but thought I'd do this after some testing.

> Also, I think we can call the function from just two places (not
> three).

Are you sure we can remove that 3rd place?  It dates back to initial
revision from 1992.  And I can't tell why it's there and what it does.

Filipp

bug#33154: 27.0.50; create_process on Darwin should not invoke setsid() after vfork() [PATCH]

[Paul Eggert]

Filipp Gunbin wrote:
> Are you sure we can remove that 3rd place?  It dates back to initial
> revision from 1992.  And I can't tell why it's there and what it does.

It's there to dissociate the controlling tty. And it's not removed, it's just 
moved into the previous call to dissociate_controlling_tty (when setsid fails).

It is a little disconcerting to change code this old. But we needn't worry about 
how it would run on 4.3BSD, only on current platforms. On most current platforms 
setsid suffices because POSIX says it should; on Darwin (and perhaps a few other 
BSD-derived systems) Emacs can fall back on TIOCNOTTY when setsid fails; and the 
proposed code does this more systematically than the current master does.

bug#33154: 27.0.50; create_process on Darwin should not invoke setsid() after vfork() [PATCH]

[Alan Third]

On Tue, Nov 06, 2018 at 11:41:46PM -0800, Paul Eggert wrote:
> Filipp Gunbin wrote:
> > In your patch, we don't detach from current (Emacs's) controlling
> > terminal before doing TIOCSCTTY.
> 
> Ah, OK. I see also that vfork won't work on Darwin if pty mode is used,
> since Emacs wants to create a new session and Darwin setsid always fails in
> a vforked child that has not yet execed.
> 
> However, your patch introduces another duplicate of the open/TIOCNOTTY/close
> fallback code, making three duplicates in all. How about if we coalesce
> these duplicates into a function and then call that function? Also, I think
> we can call the function from just two places (not three). Furthermore, I
> think it'd be more robust if Emacs does setsid everywhere (with a fallback
> to open/TIOCNOTTY/close everywhere TIOCNOTTY is available), not just Darwin.
> Proposed patch (against master) attached.

I only have two tests I know of to try here and they both pass with
this patch:

1. M‐x shell RET bg REST

doesn’t report that there’s no job control.

2. (benchmark 1 '(call-process "/usr/bin/true" nil nil nil))

Returns times in the order of 3ms, which is what we’d expect to see.

I’m not even sure if they’re really relevant, tbh. Is there anything
else I should try?
-- 
Alan Third

bug#33154: 27.0.50; create_process on Darwin should not invoke setsid() after vfork() [PATCH]

[Filipp Gunbin]

On 09/11/2018 00:07 +0000, Alan Third wrote:

> On Tue, Nov 06, 2018 at 11:41:46PM -0800, Paul Eggert wrote:
>> Filipp Gunbin wrote:
>> > In your patch, we don't detach from current (Emacs's) controlling
>> > terminal before doing TIOCSCTTY.
>>
>> Ah, OK. I see also that vfork won't work on Darwin if pty mode is used,
>> since Emacs wants to create a new session and Darwin setsid always fails in
>> a vforked child that has not yet execed.
>>
>> However, your patch introduces another duplicate of the open/TIOCNOTTY/close
>> fallback code, making three duplicates in all. How about if we coalesce
>> these duplicates into a function and then call that function? Also, I think
>> we can call the function from just two places (not three). Furthermore, I
>> think it'd be more robust if Emacs does setsid everywhere (with a fallback
>> to open/TIOCNOTTY/close everywhere TIOCNOTTY is available), not just Darwin.
>> Proposed patch (against master) attached.
>
> I only have two tests I know of to try here and they both pass with
> this patch:
>
> 1. M‐x shell RET bg REST
>
> doesn’t report that there’s no job control.
>
> 2. (benchmark 1 '(call-process "/usr/bin/true" nil nil nil))
>
> Returns times in the order of 3ms, which is what we’d expect to see.
>
> I’m not even sure if they’re really relevant, tbh. Is there anything
> else I should try?

I have one failing case: "M-x shell-command sudo ls -la" reports "sudo:
no tty present and no askpass program specified".  I don't know what to
do about it yet, but we can defer this for later.

Filipp

bug#33154: 27.0.50; create_process on Darwin should not invoke setsid() after vfork() [PATCH]

[Andreas Schwab]

On Nov 09 2018, Filipp Gunbin <fgunbin@fastmail.fm> wrote:

> I have one failing case: "M-x shell-command sudo ls -la" reports "sudo:
> no tty present and no askpass program specified".

That's not a bug.  If shell-command runs the command synchronously it
doesn't allocate a tty.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

bug#33154: 27.0.50; create_process on Darwin should not invoke setsid() after vfork() [PATCH]

[Filipp Gunbin]

On 09/11/2018 12:16 +0100, Andreas Schwab wrote:

> On Nov 09 2018, Filipp Gunbin <fgunbin@fastmail.fm> wrote:
>
>> I have one failing case: "M-x shell-command sudo ls -la" reports "sudo:
>> no tty present and no askpass program specified".
>
> That's not a bug.  If shell-command runs the command synchronously it
> doesn't allocate a tty.

Are there any reasons for not allocating a pty in this case?  A
synchronous program may be interactive.

bug#33154: 27.0.50; create_process on Darwin should not invoke setsid() after vfork() [PATCH]

[Paul Eggert]

Filipp Gunbin wrote:
> On 09/11/2018 12:16 +0100, Andreas Schwab wrote:
> 
>> On Nov 09 2018, Filipp Gunbin <fgunbin@fastmail.fm> wrote:
>>
>>> I have one failing case: "M-x shell-command sudo ls -la" reports "sudo:
>>> no tty present and no askpass program specified".
>>
>> That's not a bug.  If shell-command runs the command synchronously it
>> doesn't allocate a tty.
> 
> Are there any reasons for not allocating a pty in this case?  A
> synchronous program may be interactive.

How would the interaction work, though? I can see potential problems with 
existing Lisp code that runs synchronous commands if we allocate ptys for them.

At any rate, that case has always failed the same way on Ubuntu 18.04 etc., so 
if this is a problem it's not limited to macOS and someone should file a new bug 
report for it.

bug#33154: 27.0.50; create_process on Darwin should not invoke setsid() after vfork() [PATCH]

[Filipp Gunbin]

On 10/11/2018 09:09 -0800, Paul Eggert wrote:

> Filipp Gunbin wrote:
>> On 09/11/2018 12:16 +0100, Andreas Schwab wrote:
>>
>>> On Nov 09 2018, Filipp Gunbin <fgunbin@fastmail.fm> wrote:
>>>
>>>> I have one failing case: "M-x shell-command sudo ls -la" reports "sudo:
>>>> no tty present and no askpass program specified".
>>>
>>> That's not a bug.  If shell-command runs the command synchronously it
>>> doesn't allocate a tty.
>>
>> Are there any reasons for not allocating a pty in this case?  A
>> synchronous program may be interactive.
>
> How would the interaction work, though? I can see potential problems with
> existing Lisp code that runs synchronous commands if we allocate ptys
> for them.

Yes, I see, there's no way to interact.  Without interaction, there's no
sense in allocating pty.

> At any rate, that case has always failed the same way on Ubuntu 18.04 etc., so
> if this is a problem it's not limited to macOS and someone should file a new bug
> report for it.

No need for that.

Thanks.

bug#33154: 27.0.50; create_process on Darwin should not invoke setsid() after vfork() [PATCH]

[Paul Eggert]

Alan Third wrote:

> I only have two tests I know of to try here and they both pass

Thanks for checking. I installed the patch on the master branch and am marking 
this bug as done.