Use and prefer os.posix_spawn() when available

[cagney]

Python 3.8 has added os.posix_spawn(), this changes ptyprocess
to use it when available.

Since os.posix_spawn() completey bypasses os.fork(), pty.fork(),
it avoids problems with logging locks and code needing to be thread-safe.

[takluyver]

Thanks, this is interesting. I have a couple of concerns, though. Most significantly, the functions to fork a process into a pty all have some extra code to make the new tty the controlling tty of the new process. Here's ptyprocess' fallback:

ptyprocess/ptyprocess/_fork_pty.py

Line 31 in 3931cd4

pty_make_controlling_tty(child_fd)

Here it is in Python's stdlib:

https://github.com/python/cpython/blob/9e4f2f3a6b8ee995c365e86d976937c141d867f8/Lib/pty.py#L110-L112

And here it is in glibc's forkpty function:

https://github.com/lattera/glibc/blob/895ef79e04a953cac1493863bcae29ad85657ee1/login/forkpty.c#L43-L44

https://github.com/lattera/glibc/blob/895ef79e04a953cac1493863bcae29ad85657ee1/login/login_tty.c

I don't think the posix_spawn implementation does anything equivalent to this, and I suspect it's important.

Secondly, the posix_spawn call ultimately seems to use fork() inside glibc:

https://github.com/lattera/glibc/blob/895ef79e04a953cac1493863bcae29ad85657ee1/sysdeps/posix/spawni.c#L280-L281

Presumably the glibc implementation is pretty good, but I'd still have some concern that we're trading one set of problems for another. The possibility that the whole block needs wrapping in a Python-level threading lock doesn't inspire confidence - even if it ultimately turns out not to need that, the fact that it looks like it might isn't great.

[cagney]

Thanks, this is interesting. I have a couple of concerns, though. Most significantly, the functions to fork a process into a pty all have some extra code to make the new tty the controlling tty of the new process. Here's ptyprocess' fallback:

https://github.com/lattera/glibc/blob/895ef79e04a953cac1493863bcae29ad85657ee1/login/login_tty.c

I don't think the posix_spawn implementation does anything equivalent to this, and I suspect it's important.

Yea. Below are extracts from a BSD implementation of login_tty() (its the same as glibc, but simpler):

   (void) setsid();

For screwing around with the process group, posix_spawn() seems to have two mechanisms:
setpgroup (setpgrp(2))
setsid (setsid(2)) - I pass setsid=True
However, looking more carefully I see that telling posix_spawn() to call setsid() is a GNU extension. I'll change things to deal with this.

    if (ioctl(fd, TIOCSCTTY, NULL) == -1)
            return (-1);

             Make the terminal the controlling terminal for the process
             (the process must not currently have a controlling terminal).

It seems to be missing. I'm not seeing problems, but then I'm not trying to run something like an interactive bash in the sub-process. I'll look into it.

    (void) dup2(fd, STDIN_FILENO);
    (void) dup2(fd, STDOUT_FILENO);
    (void) dup2(fd, STDERR_FILENO);
    if (fd != STDIN_FILENO && fd != STDOUT_FILENO && fd != STDERR_FILENO)
            (void) close(fd);

this is handled by the file_actions=... argument.

Secondly, the posix_spawn call ultimately seems to use fork() inside glibc:

BTW, on Solaris and NetBSD, at least, it's implemented as a system call.

https://github.com/lattera/glibc/blob/895ef79e04a953cac1493863bcae29ad85657ee1/sysdeps/posix/spawni.c#L280-L281

Presumably the glibc implementation is pretty good, but I'd still have some concern that we're trading one set of problems for another. The possibility that the whole block needs wrapping in a Python-level threading lock doesn't inspire confidence - even if it ultimately turns out not to need that, the fact that it looks like it might isn't great.

The problem's with os.openpty() / openpty(3) which returns the PTY/TTY without O_CLOEXEC set. This means that fork()/exec() in a separate thread would have the FD left open :-(

However, this also means I can simplify things and just wrap the os.openpty() and setting non-inheritable.

[takluyver]

Having done some more reading, I still think the missing 'controlling tty' part is important. It looks like this may be used for:

• Sending signals from the terminal to processes running inside it
• (Possibly) Job control, foreground & background processes
• Some programs read directly from their controlling terminal to bypass stdin for things like password prompts.

I also looked into the source code of libvte, the terminal emulator library used by Gnome terminal (among other applications). As best I can tell, it also uses fork() followed by setup steps in the child process.

As attractive as it is, I think posix_spawn is a dead end for this purpose. If you want to make the spawning process more robust, I think you'll need to write C code to do the fork+pty setup+exec safely, and either distribute it as a new extension module or contribute it to the standard library.