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

3.1.0 switch to rustix again for glibc compat #32

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ libc = "0.2.152"
# Default enable napi4 feature, see https://nodejs.org/api/n-api.html#node-api-version-matrix
napi = { version = "2.12.2", default-features = false, features = ["napi4"] }
napi-derive = "2.12.2"
nix = { version = "0.29.0", features = ["fs", "term", "poll"] }
rustix = { version = "0.38.34", features = ["event", "termios"] }
rustix-openpty = "0.1.1"

[build-dependencies]
napi-build = "2.0.1"
Expand Down
2 changes: 1 addition & 1 deletion index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export class Pty {
* Returns a file descriptor for the PTY controller.
* See the docstring of the class for an usage example.
*/
fd(): c_int
fd(): number
/**
* Close the PTY file descriptor. This must be called when the readers / writers of the PTY have
* been closed, otherwise we will leak file descriptors!
Expand Down
49 changes: 25 additions & 24 deletions src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
use backoff::backoff::Backoff;
use backoff::ExponentialBackoffBuilder;
use libc::{self, c_int};
use napi::bindgen_prelude::JsFunction;
use napi::threadsafe_function::{ErrorStrategy, ThreadsafeFunction, ThreadsafeFunctionCallMode};
use napi::Status::GenericFailure;
use napi::{self, Env};
use nix::errno::Errno;
use nix::poll::{poll, PollFd, PollFlags, PollTimeout};
use nix::pty::{openpty, Winsize};
use nix::sys::termios::{self, SetArg};
use rustix::event::{poll, PollFd, PollFlags};
use rustix::io::Errno;
use rustix::termios::{self, InputModes, OptionalActions, Winsize};
use rustix_openpty::openpty;
use std::collections::HashMap;
use std::io::Error;
use std::io::ErrorKind;
use std::os::fd::FromRawFd;
use std::os::fd::{AsRawFd, OwnedFd};
use std::os::fd::{BorrowedFd, FromRawFd, RawFd};
use std::os::unix::process::CommandExt;
use std::process::{Command, Stdio};
use std::thread;
Expand Down Expand Up @@ -56,20 +55,22 @@ fn cast_to_napi_error(err: Errno) -> napi::Error {
// if the child process exits before the controller fd is fully read, we might accidentally
// end in a case where onExit is called but js hasn't had the chance to fully read the controller fd
// let's wait until the controller fd is fully read before we call onExit
fn poll_controller_fd_until_read(raw_fd: RawFd) {
fn poll_controller_fd_until_read(controller_fd: OwnedFd) {
// wait until fd is fully read (i.e. POLLIN no longer set)
let borrowed_fd = unsafe { BorrowedFd::borrow_raw(raw_fd) };
let poll_fd = PollFd::new(borrowed_fd, PollFlags::POLLIN);

let mut poll_fds = [PollFd::new(&controller_fd, PollFlags::IN)];
let mut backoff = ExponentialBackoffBuilder::default()
.with_initial_interval(Duration::from_millis(1))
.with_max_interval(Duration::from_millis(100))
.with_max_elapsed_time(Some(Duration::from_secs(1)))
.build();

loop {
if let Err(err) = poll(&mut [poll_fd], PollTimeout::ZERO) {
if err == Errno::EINTR || err == Errno::EAGAIN {
for poll_fd in &mut poll_fds[..] {
poll_fd.clear_revents();
}

if let Err(err) = poll(&mut poll_fds, 0) {
if err == Errno::INTR || err == Errno::AGAIN {
// we were interrupted, so we should just try again
continue;
}
Expand All @@ -79,10 +80,9 @@ fn poll_controller_fd_until_read(raw_fd: RawFd) {
}

// check if POLLIN is no longer set (i.e. there is no more data to read)
if let Some(flags) = poll_fd.revents() {
if !flags.contains(PollFlags::POLLIN) {
break;
}
let flags = poll_fds[0].revents();
if !flags.contains(PollFlags::IN) {
break;
}

// wait for a bit before trying again
Expand Down Expand Up @@ -115,9 +115,9 @@ impl Pty {
}

// open pty pair
let pty_res = openpty(&window_size, None).map_err(cast_to_napi_error)?;
let controller_fd = pty_res.master;
let user_fd = pty_res.slave;
let pty_res = openpty(None, Some(&window_size)).map_err(cast_to_napi_error)?;
let controller_fd = pty_res.controller;
let user_fd = pty_res.user;

// duplicate pty user_fd to be the child's stdin, stdout, and stderr
cmd.stdin(Stdio::from(user_fd.try_clone()?));
Expand Down Expand Up @@ -160,8 +160,8 @@ impl Pty {
// set input modes
let user_fd = OwnedFd::from_raw_fd(raw_user_fd);
if let Ok(mut termios) = termios::tcgetattr(&user_fd) {
termios.input_flags |= termios::InputFlags::IUTF8;
termios::tcsetattr(&user_fd, SetArg::TCSANOW, &termios)?;
termios.input_modes.set(InputModes::IUTF8, true);
termios::tcsetattr(&user_fd, OptionalActions::Now, &termios)?;
}

// reset signal handlers
Expand Down Expand Up @@ -197,11 +197,12 @@ impl Pty {
.on_exit
.create_threadsafe_function(0, |ctx| ctx.env.create_int32(ctx.value).map(|v| vec![v]))?;

let cloned_controller_fd = controller_fd.try_clone()?;
thread::spawn(move || {
let wait_result = child.wait();

// try to wait for the controller fd to be fully read
poll_controller_fd_until_read(raw_controller_fd);
// try to wait for the controller fd to be fully read
poll_controller_fd_until_read(cloned_controller_fd);

// we don't drop the controller fd immediately
// let pty.close() be responsible for closing it
Expand Down Expand Up @@ -272,7 +273,7 @@ impl Pty {
/// See the docstring of the class for an usage example.
#[napi]
#[allow(dead_code)]
pub fn fd(&mut self) -> Result<c_int, napi::Error> {
pub fn fd(&mut self) -> Result<i32, napi::Error> {
if let Some(fd) = &self.controller_fd {
Ok(fd.as_raw_fd())
} else {
Expand Down
Loading