PTY.spawn does not set new controlling tty for child

[headius]

As part of work to get our FFI-based io/console green (ruby/io-console#22) I discovered that our PTY module seems to break the subprocess io/console tests. This appears to be because our current PTY.spawn implementation is unable to properly set the new controlling tty, and as a result the raw IO operations in the child are routed back to the parent process.

This leads to several tests hanging on JRuby, since output they expect to read never arrives.

The PTY implementation originally came from @Freaky and has not had many changes since: #3185

The code in CRuby for this logic performs a number of operations in the child before proceeding to exec, and I assume these are the missing pieces needed to grant the child its own tty: https://github.com/ruby/ruby/blob/3108ad7bf3dcae52054a1c29b86246cdb470000b/ext/pty/pty.c#L102-L165

At this point, I do not know how to fix this, so it is a known bug in our PTY module.

One solution might be to finally start shipping a minimal executable that we can use to fork + exec and do these various operations in the child, as a sort of super-posix_spawn. No plans to work on that currently.

[sionescu]

On *nix platforms, this could be solved by using a C library of mine, libfixposix, which has an API similar to posix_spawn but with more operations. See lfp_spawnattr_setctty.

That library won't work on Windows, but it seems that Windows now has support for PTYs since 2018, and only on Windows 10.

[headius]

@sionescu Thank you for reaching out! Your library does indeed look like an excellent set of enhancements for posix_spawn and I will be looking into how we can make use of it. Simplest way might be for us to package it as a Ruby gem that builds it on install and binds it with FFI. Then any Ruby implementation can take advantage of the enhanced spawn capabilities.

[sionescu]

I'll try to get a release done soon, then come up with a gem that I'd like you to review since I haven't done Ruby since MRuby 1.8 or so.

[headius]

@sionescu Oh that would be awesome! We could build a library around it that works the same on all Ruby impls but provides the missing pieces of posix_spawn. Keep me posted, I want like to help but my skills in building and packaging native libraries is limited.

[sionescu]

@headius Ok, very good. What's your primary OS ? I develop libfixposix on Linux, and although I'm reasonably confident that the library should work on OSX and the BSDs, it's only lightly tested there.

[headius]

@sionescu I develop on OSX, @enebo and @kares are on Linux. As an optional gem I am not worried... folks that really need the enhanced spawn capabilities on JRuby could just install the gem and then get full PTY etc support.

[byteit101]

@sionescu I've been working on integrating libfixposix with JRuby (current progress at https://github.com/byteit101/lfp-process-builder for the moment) and am wondering if you are interested in adding rlimit and umask options to the lfp_spawnattr_* set ? I'm hoping to be able to support a superset of Process.spawn with this work. The only other thing missing is close_others, but as we are in the JVM, I think we can just close_on_exec them all, and don't require libfixposix's help there?

[headius]

close_others requires that we can maintain a list of all file descriptors open as in CRuby, I believe, and given the JVM manages it's own descriptors for most IO it is not really feasible. However, most fd are being opened with CLOEXEC so usually we will not leak too many descriptors to the child process.

Having rlimit and umask would be great 😃

[sionescu]

@byteit101 Yes, I've opened the bugs for adding those two spawn attributes. As for close_others, that's an OSX extension called POSIX_SPAWN_CLOEXEC_DEFAULT and I've opened a bug about that as well.
@headius You don't need to maintain a list of all file descriptors, you just need to know which ones you want the child process to inherit, and set CLOEXEC on all the rest.

[byteit101]

@sionescu sweet! After further research I think everything is cloexec by default in all the places we care about, so the OSX extension is cool but not needed by us

[headius]

which ones you want the child process to inherit, and set CLOEXEC on all the rest

Does this not mean we'd have to know all file descriptors? [descriptors for the child] ∪ [all the rest] = [all the descriptors]

We do not have a way to get a list of all open descriptors that the JVM has opened (but hopefully they are all CLOEXEC).

[sionescu]

It means you'd only have to specify the descriptors you want the child to inherit, and the library can mass-CLOEXEC all the rest. Because this can be somewhat slow depending on the method of iterating over descriptors (still we're talking about ~1ms), it should be opt-in, hence the POSIX_SPAWN_CLOEXEC_DEFAULT option that I'd like to implement.

[headius]

Aha, now I understand. Thanks!

[byteit101]

@sionescu I am hoping to have some initial releases of the ffi gems soon, do you think you will have time to review, merge, and release a new version of libfixposix with my changes soon, or should we have a temporary soft-fork while you do that later? If the latter, I think I'll mark the version as 0.4.9?

[sionescu]

@byteit101 I'll merge that functionality and make a release at the end of the week.

[sionescu]

Release 0.5.0 is now live and passes tests on Ubuntu 22.04, OSX/Xcode14.0 and Freebsd 12.1: https://app.travis-ci.com/github/sionescu/libfixposix/builds/257627545

[byteit101]

Awesome, I'll get to work publishing the low level gems after I remove the uid/gid references after the change I see you made!