refactor(ext/io): move tty metadata to separate collection (#18959)

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.

ext/io/lib.rs

@@ -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;
@@ -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;
@@ -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>,
@@ -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,
@@ -433,10 +403,7 @@ pub struct StdFileResource {
 impl StdFileResource {
   fn stdio(inner: StdFileResourceInner, name: &str) -> Self {
     Self {
-      cell: RefCell::new(Some(StdFileResourceCellValue {
+      cell: RefCell::new(Some(inner)),
-        inner,
-        meta_data: Default::default(),
-      })),
       cell_async_task_queue: Default::default(),
       name: name.to_string(),
     }
@@ -444,26 +411,20 @@ impl StdFileResource {
 
   pub fn fs_file(fs_file: StdFile) -> Self {
     Self {
-      cell: RefCell::new(Some(StdFileResourceCellValue {
+      cell: RefCell::new(Some(StdFileResourceInner::file(fs_file))),
-        inner: StdFileResourceInner::file(fs_file),
-        meta_data: Default::default(),
-      })),
       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(
+    action: impl FnOnce(&mut StdFileResourceInner) -> Result<TResult, E>,
-      &mut StdFileResourceInner,
-      &Arc<Mutex<FileMetadata>>,
-    ) -> 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)
       }
@@ -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
@@ -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)?
   }
 
@@ -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)?
     })
   }
@@ -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))
+    self.with_inner(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)?
-    })
   }
 
   pub async fn with_file_blocking_task<F, R: Send + 'static>(
@@ -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())
@@ -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()
@@ -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(())
       })

runtime/ops/tty.rs

@@ -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;
@@ -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();
   },
@@ -118,22 +149,21 @@ fn op_stdin_set_raw(
   {
     use std::os::unix::io::AsRawFd;
 
-    StdFileResource::with_file_and_metadata(
+    let tty_mode_store = state.borrow::<TtyModeStore>().clone();
-      state,
+    let previous_mode = tty_mode_store.get(rid);
-      rid,
+
-      move |std_file, meta_data| {
+    StdFileResource::with_file(state, rid, move |std_file| {
       let raw_fd = std_file.as_raw_fd();
 
       if is_raw {
-          let mut raw = {
+        let mut raw = match previous_mode {
-            let mut meta_data = meta_data.lock();
+          Some(mode) => mode,
-            let maybe_tty_mode = &mut meta_data.tty.mode;
+          None => {
-            if maybe_tty_mode.is_none() {
             // Save original mode.
             let original_mode = termios::tcgetattr(raw_fd)?;
-              maybe_tty_mode.replace(original_mode);
+            tty_mode_store.set(rid, original_mode.clone());
+            original_mode
           }
-            maybe_tty_mode.clone().unwrap()
         };
 
         raw.input_flags &= !(termios::InputFlags::BRKINT
@@ -150,21 +180,18 @@ fn op_stdin_set_raw(
         if !cbreak {
           raw.local_flags &= !(termios::LocalFlags::ISIG);
         }
-          raw.control_chars[termios::SpecialCharacterIndices::VMIN as usize] =
+        raw.control_chars[termios::SpecialCharacterIndices::VMIN as usize] = 1;
-            1;
+        raw.control_chars[termios::SpecialCharacterIndices::VTIME as usize] = 0;
-          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() {
+        if let Some(mode) = tty_mode_store.take(rid) {
           termios::tcsetattr(raw_fd, termios::SetArg::TCSADRAIN, &mode)?;
         }
       }
 
       Ok(())
-      },
+    })
-    )
   }
 }