Skip to content

Commit

Permalink
refactor(ext/io): move tty metadata to separate collection (#18959)
Browse files Browse the repository at this point in the history 
This removes the tty stuff that's hanging on the file resources and
instead stores them in a separate `TtyModeStore`. Although this will
cause the tty store items to not be removed when the resource is
removed, I think this is ok to do because there will be a small number
of resources this is every done with and usually those resources won't
ever be closed.
  • Loading branch information
@dsherret @levex
dsherret authored and levex committed May 4, 2023
1 parent 4ab6c67 commit 6f36535
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 114 deletions.
84 changes: 14 additions & 70 deletions ext/io/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
use deno_core::error::resource_unavailable;
use deno_core::error::AnyError;
use deno_core::op;
use deno_core::parking_lot::Mutex;
use deno_core::AsyncMutFuture;
use deno_core::AsyncRefCell;
use deno_core::AsyncResult;
Expand All @@ -25,7 +24,6 @@ use std::io::ErrorKind;
use std::io::Read;
use std::io::Write;
use std::rc::Rc;
use std::sync::Arc;
use tokio::io::AsyncRead;
use tokio::io::AsyncReadExt;
use tokio::io::AsyncWrite;
Expand Down Expand Up @@ -159,20 +157,6 @@ pub struct Stdio {
pub stderr: StdioPipe,
}

#[cfg(unix)]
use nix::sys::termios;

#[derive(Default)]
pub struct TtyMetadata {
#[cfg(unix)]
pub mode: Option<termios::Termios>,
}

#[derive(Default)]
pub struct FileMetadata {
pub tty: TtyMetadata,
}

#[derive(Debug)]
pub struct WriteOnlyResource<S> {
stream: AsyncRefCell<S>,
Expand Down Expand Up @@ -405,26 +389,12 @@ impl Read for StdFileResourceInner {
}
}

struct StdFileResourceCellValue {
inner: StdFileResourceInner,
meta_data: Arc<Mutex<FileMetadata>>,
}

impl StdFileResourceCellValue {
pub fn try_clone(&self) -> Result<Self, std::io::Error> {
Ok(Self {
inner: self.inner.try_clone()?,
meta_data: self.meta_data.clone(),
})
}
}

pub struct StdFileResource {
name: String,
// We can't use an AsyncRefCell here because we need to allow
// access to the resource synchronously at any time and
// asynchronously one at a time in order
cell: RefCell<Option<StdFileResourceCellValue>>,
cell: RefCell<Option<StdFileResourceInner>>,
// Used to keep async actions in order and only allow one
// to occur at a time
cell_async_task_queue: TaskQueue,
Expand All @@ -433,37 +403,28 @@ pub struct StdFileResource {
impl StdFileResource {
fn stdio(inner: StdFileResourceInner, name: &str) -> Self {
Self {
cell: RefCell::new(Some(StdFileResourceCellValue {
inner,
meta_data: Default::default(),
})),
cell: RefCell::new(Some(inner)),
cell_async_task_queue: Default::default(),
name: name.to_string(),
}
}

pub fn fs_file(fs_file: StdFile) -> Self {
Self {
cell: RefCell::new(Some(StdFileResourceCellValue {
inner: StdFileResourceInner::file(fs_file),
meta_data: Default::default(),
})),
cell: RefCell::new(Some(StdFileResourceInner::file(fs_file))),
cell_async_task_queue: Default::default(),
name: "fsFile".to_string(),
}
}

fn with_inner_and_metadata<TResult, E>(
fn with_inner<TResult, E>(
&self,
action: impl FnOnce(
&mut StdFileResourceInner,
&Arc<Mutex<FileMetadata>>,
) -> Result<TResult, E>,
action: impl FnOnce(&mut StdFileResourceInner) -> Result<TResult, E>,
) -> Option<Result<TResult, E>> {
match self.cell.try_borrow_mut() {
Ok(mut cell) if cell.is_some() => {
let mut file = cell.take().unwrap();
let result = action(&mut file.inner, &file.meta_data);
let result = action(&mut file);
cell.replace(file);
Some(result)
}
Expand Down Expand Up @@ -491,7 +452,7 @@ impl StdFileResource {
}
};
let (cell_value, result) = tokio::task::spawn_blocking(move || {
let result = action(&mut cell_value.inner);
let result = action(&mut cell_value);
(cell_value, result)
})
.await
Expand Down Expand Up @@ -539,14 +500,14 @@ impl StdFileResource {

fn read_byob_sync(self: Rc<Self>, buf: &mut [u8]) -> Result<usize, AnyError> {
self
.with_inner_and_metadata(|inner, _| inner.read(buf))
.with_inner(|inner| inner.read(buf))
.ok_or_else(resource_unavailable)?
.map_err(Into::into)
}

fn write_sync(self: Rc<Self>, data: &[u8]) -> Result<usize, AnyError> {
self
.with_inner_and_metadata(|inner, _| inner.write_and_maybe_flush(data))
.with_inner(|inner| inner.write_and_maybe_flush(data))
.ok_or_else(resource_unavailable)?
}

Expand All @@ -572,7 +533,7 @@ impl StdFileResource {
{
Self::with_resource(state, rid, move |resource| {
resource
.with_inner_and_metadata(move |inner, _| inner.with_file(f))
.with_inner(move |inner| inner.with_file(f))
.ok_or_else(resource_unavailable)?
})
}
Expand All @@ -581,24 +542,7 @@ impl StdFileResource {
where
F: FnOnce(&mut StdFile) -> Result<R, io::Error>,
{
self.with_inner_and_metadata(move |inner, _| inner.with_file(f))
}

pub fn with_file_and_metadata<F, R>(
state: &mut OpState,
rid: ResourceId,
f: F,
) -> Result<R, AnyError>
where
F: FnOnce(&mut StdFile, &Arc<Mutex<FileMetadata>>) -> Result<R, AnyError>,
{
Self::with_resource(state, rid, move |resource| {
resource
.with_inner_and_metadata(move |inner, metadata| {
inner.with_file(move |file| f(file, metadata))
})
.ok_or_else(resource_unavailable)?
})
self.with_inner(move |inner| inner.with_file(f))
}

pub async fn with_file_blocking_task<F, R: Send + 'static>(
Expand Down Expand Up @@ -646,7 +590,7 @@ impl StdFileResource {
) -> Result<std::process::Stdio, AnyError> {
Self::with_resource(state, rid, |resource| {
resource
.with_inner_and_metadata(|inner, _| match inner.kind {
.with_inner(|inner| match inner.kind {
StdFileResourceKind::File => {
let file = inner.file.try_clone()?;
Ok(file.into())
Expand Down Expand Up @@ -712,7 +656,7 @@ impl Resource for StdFileResource {
fn backing_fd(self: Rc<Self>) -> Option<std::os::unix::prelude::RawFd> {
use std::os::unix::io::AsRawFd;
self
.with_inner_and_metadata(move |std_file, _| {
.with_inner(move |std_file| {
Ok::<_, ()>(std_file.with_file(|f| f.as_raw_fd()))
})?
.ok()
Expand All @@ -729,7 +673,7 @@ pub fn op_print(
let rid = if is_err { 2 } else { 1 };
StdFileResource::with_resource(state, rid, move |resource| {
resource
.with_inner_and_metadata(|inner, _| {
.with_inner(|inner| {
inner.write_all_and_maybe_flush(msg.as_bytes())?;
Ok(())
})
Expand Down
115 changes: 71 additions & 44 deletions runtime/ops/tty.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,35 @@ use deno_core::OpState;
use deno_io::StdFileResource;
use std::io::Error;

#[cfg(unix)]
use deno_core::ResourceId;
#[cfg(unix)]
use nix::sys::termios;
#[cfg(unix)]
use std::cell::RefCell;
#[cfg(unix)]
use std::collections::HashMap;
#[cfg(unix)]
use std::rc::Rc;

#[cfg(unix)]
#[derive(Default, Clone)]
struct TtyModeStore(Rc<RefCell<HashMap<ResourceId, termios::Termios>>>);

#[cfg(unix)]
impl TtyModeStore {
pub fn get(&self, id: ResourceId) -> Option<termios::Termios> {
self.0.borrow().get(&id).map(ToOwned::to_owned)
}

pub fn take(&self, id: ResourceId) -> Option<termios::Termios> {
self.0.borrow_mut().remove(&id)
}

pub fn set(&self, id: ResourceId, mode: termios::Termios) {
self.0.borrow_mut().insert(id, mode);
}
}

#[cfg(windows)]
use deno_core::error::custom_error;
Expand Down Expand Up @@ -35,6 +62,10 @@ fn get_windows_handle(
deno_core::extension!(
deno_tty,
ops = [op_stdin_set_raw, op_isatty, op_console_size],
state = |state| {
#[cfg(unix)]
state.put(TtyModeStore::default());
},
customizer = |ext: &mut deno_core::ExtensionBuilder| {
ext.force_op_registration();
},
Expand Down Expand Up @@ -118,53 +149,49 @@ fn op_stdin_set_raw(
{
use std::os::unix::io::AsRawFd;

StdFileResource::with_file_and_metadata(
state,
rid,
move |std_file, meta_data| {
let raw_fd = std_file.as_raw_fd();

if is_raw {
let mut raw = {
let mut meta_data = meta_data.lock();
let maybe_tty_mode = &mut meta_data.tty.mode;
if maybe_tty_mode.is_none() {
// Save original mode.
let original_mode = termios::tcgetattr(raw_fd)?;
maybe_tty_mode.replace(original_mode);
}
maybe_tty_mode.clone().unwrap()
};

raw.input_flags &= !(termios::InputFlags::BRKINT
| termios::InputFlags::ICRNL
| termios::InputFlags::INPCK
| termios::InputFlags::ISTRIP
| termios::InputFlags::IXON);

raw.control_flags |= termios::ControlFlags::CS8;

raw.local_flags &= !(termios::LocalFlags::ECHO
| termios::LocalFlags::ICANON
| termios::LocalFlags::IEXTEN);
if !cbreak {
raw.local_flags &= !(termios::LocalFlags::ISIG);
}
raw.control_chars[termios::SpecialCharacterIndices::VMIN as usize] =
1;
raw.control_chars[termios::SpecialCharacterIndices::VTIME as usize] =
0;
termios::tcsetattr(raw_fd, termios::SetArg::TCSADRAIN, &raw)?;
} else {
// Try restore saved mode.
if let Some(mode) = meta_data.lock().tty.mode.take() {
termios::tcsetattr(raw_fd, termios::SetArg::TCSADRAIN, &mode)?;
let tty_mode_store = state.borrow::<TtyModeStore>().clone();
let previous_mode = tty_mode_store.get(rid);

StdFileResource::with_file(state, rid, move |std_file| {
let raw_fd = std_file.as_raw_fd();

if is_raw {
let mut raw = match previous_mode {
Some(mode) => mode,
None => {
// Save original mode.
let original_mode = termios::tcgetattr(raw_fd)?;
tty_mode_store.set(rid, original_mode.clone());
original_mode
}
};

raw.input_flags &= !(termios::InputFlags::BRKINT
| termios::InputFlags::ICRNL
| termios::InputFlags::INPCK
| termios::InputFlags::ISTRIP
| termios::InputFlags::IXON);

raw.control_flags |= termios::ControlFlags::CS8;

raw.local_flags &= !(termios::LocalFlags::ECHO
| termios::LocalFlags::ICANON
| termios::LocalFlags::IEXTEN);
if !cbreak {
raw.local_flags &= !(termios::LocalFlags::ISIG);
}
raw.control_chars[termios::SpecialCharacterIndices::VMIN as usize] = 1;
raw.control_chars[termios::SpecialCharacterIndices::VTIME as usize] = 0;
termios::tcsetattr(raw_fd, termios::SetArg::TCSADRAIN, &raw)?;
} else {
// Try restore saved mode.
if let Some(mode) = tty_mode_store.take(rid) {
termios::tcsetattr(raw_fd, termios::SetArg::TCSADRAIN, &mode)?;
}
}

Ok(())
},
)
Ok(())
})
}
}

Expand Down

0 comments on commit 6f36535

Please sign in to comment.