Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite PTY support #3235

Closed
wants to merge 4 commits into from
Closed

Rewrite PTY support #3235

wants to merge 4 commits into from

Conversation

Freaky
Copy link
Contributor

@Freaky Freaky commented Aug 7, 2015

This reworks PTY#spawn to use openpty(3) (available on BSD, OS X and Linux) and Process#spawn instead of forkpty(3), which as the name suggests, forks and is thus unsafe. It also fixes use with multiple arguments.

It also adds PTY#open and PTY#check, and properly defines PTY#getpty as an alias to PTY#spawn.

It passes MRI's test_pty.rb, after a slight tweak to close a race condition:

Finished tests in 15.752000s, 0.9523 tests/s, 2.4759 assertions/s.
15 tests, 39 assertions, 0 failures, 0 errors, 0 skips

ruby -v: jruby 9.0.1.0-SNAPSHOT (2.2.2) 2015-08-01 6a9fcf8 OpenJDK 64-Bit Server VM 25.51-b03 on 1.8.0_51-b16 +jit [FreeBSD-amd64]

Needs testing and review, especially outside FreeBSD.

@Freaky
Copy link
Contributor Author

Freaky commented Aug 7, 2015

Still issues with this. e.g. spawning off a copy of yes and closing both descriptors doesn't send it a SIGHUP. cat works, though, or the test suite would hang.

@Freaky
Copy link
Contributor Author

Freaky commented Apr 2, 2017

Closing - committed in 4f8e6bd

@Freaky Freaky closed this Apr 2, 2017
@headius headius added this to the Invalid or Duplicate milestone Apr 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants