Skip to content

Commit

Permalink
refactor: remove unused Options on StdFileResource.fs_file (denol…
Browse files Browse the repository at this point in the history 
…and#14549)
  • Loading branch information
@dsherret
dsherret authored May 10, 2022
1 parent 64f9711 commit 11c13fb
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 127 deletions.
68 changes: 16 additions & 52 deletions runtime/ops/fs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use super::io::StdFileResource;
use super::utils::into_string;
use crate::fs_util::canonicalize_path;
use crate::permissions::Permissions;
use deno_core::error::bad_resource_id;
use deno_core::error::custom_error;
use deno_core::error::type_error;
use deno_core::error::AnyError;
Expand All @@ -24,11 +23,16 @@ use serde::Serialize;
use std::borrow::Cow;
use std::cell::RefCell;
use std::convert::From;
use std::env::{current_dir, set_current_dir, temp_dir};
use std::env::current_dir;
use std::env::set_current_dir;
use std::env::temp_dir;
use std::io;
use std::io::Error;
use std::io::Seek;
use std::io::SeekFrom;
use std::io::Write;
use std::io::{Error, Seek, SeekFrom};
use std::path::{Path, PathBuf};
use std::path::Path;
use std::path::PathBuf;
use std::rc::Rc;
use std::time::SystemTime;
use std::time::UNIX_EPOCH;
Expand Down Expand Up @@ -339,12 +343,7 @@ async fn op_seek_async(
.resource_table
.get::<StdFileResource>(rid)?;

if resource.fs_file.is_none() {
return Err(bad_resource_id());
}

let fs_file = resource.fs_file.as_ref().unwrap();
let std_file = fs_file.0.as_ref().unwrap().clone();
let std_file = resource.std_file()?;

tokio::task::spawn_blocking(move || {
let mut std_file = std_file.lock().unwrap();
Expand Down Expand Up @@ -376,12 +375,7 @@ async fn op_fdatasync_async(
.resource_table
.get::<StdFileResource>(rid)?;

if resource.fs_file.is_none() {
return Err(bad_resource_id());
}

let fs_file = resource.fs_file.as_ref().unwrap();
let std_file = fs_file.0.as_ref().unwrap().clone();
let std_file = resource.std_file()?;

tokio::task::spawn_blocking(move || {
let std_file = std_file.lock().unwrap();
Expand Down Expand Up @@ -410,12 +404,7 @@ async fn op_fsync_async(
.resource_table
.get::<StdFileResource>(rid)?;

if resource.fs_file.is_none() {
return Err(bad_resource_id());
}

let fs_file = resource.fs_file.as_ref().unwrap();
let std_file = fs_file.0.as_ref().unwrap().clone();
let std_file = resource.std_file()?;

tokio::task::spawn_blocking(move || {
let std_file = std_file.lock().unwrap();
Expand Down Expand Up @@ -447,12 +436,7 @@ async fn op_fstat_async(
.resource_table
.get::<StdFileResource>(rid)?;

if resource.fs_file.is_none() {
return Err(bad_resource_id());
}

let fs_file = resource.fs_file.as_ref().unwrap();
let std_file = fs_file.0.as_ref().unwrap().clone();
let std_file = resource.std_file()?;

let metadata = tokio::task::spawn_blocking(move || {
let std_file = std_file.lock().unwrap();
Expand Down Expand Up @@ -499,12 +483,7 @@ async fn op_flock_async(
.resource_table
.get::<StdFileResource>(rid)?;

if resource.fs_file.is_none() {
return Err(bad_resource_id());
}

let fs_file = resource.fs_file.as_ref().unwrap();
let std_file = fs_file.0.as_ref().unwrap().clone();
let std_file = resource.std_file()?;

tokio::task::spawn_blocking(move || -> Result<(), AnyError> {
let std_file = std_file.lock().unwrap();
Expand Down Expand Up @@ -548,12 +527,7 @@ async fn op_funlock_async(
.resource_table
.get::<StdFileResource>(rid)?;

if resource.fs_file.is_none() {
return Err(bad_resource_id());
}

let fs_file = resource.fs_file.as_ref().unwrap();
let std_file = fs_file.0.as_ref().unwrap().clone();
let std_file = resource.std_file()?;

tokio::task::spawn_blocking(move || -> Result<(), AnyError> {
let std_file = std_file.lock().unwrap();
Expand Down Expand Up @@ -1636,12 +1610,7 @@ async fn op_ftruncate_async(
.resource_table
.get::<StdFileResource>(rid)?;

if resource.fs_file.is_none() {
return Err(bad_resource_id());
}

let fs_file = resource.fs_file.as_ref().unwrap();
let std_file = fs_file.0.as_ref().unwrap().clone();
let std_file = resource.std_file()?;

tokio::task::spawn_blocking(move || {
let std_file = std_file.lock().unwrap();
Expand Down Expand Up @@ -1938,12 +1907,7 @@ async fn op_futime_async(
.resource_table
.get::<StdFileResource>(rid)?;

if resource.fs_file.is_none() {
return Err(bad_resource_id());
}

let fs_file = resource.fs_file.as_ref().unwrap();
let std_file = fs_file.0.as_ref().unwrap().clone();
let std_file = resource.std_file()?;
tokio::task::spawn_blocking(move || {
let std_file = std_file.lock().unwrap();
filetime::set_file_handle_times(&std_file, Some(atime), Some(mtime))?;
Expand Down
77 changes: 33 additions & 44 deletions runtime/ops/io.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,70 +301,66 @@ impl Resource for ChildStderrResource {
}
}

type MaybeSharedStdFile = Option<Arc<Mutex<StdFile>>>;

#[derive(Default)]
pub struct StdFileResource {
pub fs_file: Option<(MaybeSharedStdFile, Option<RefCell<FileMetadata>>)>,
fs_file: Option<Arc<Mutex<StdFile>>>,
metadata: RefCell<FileMetadata>,
cancel: CancelHandle,
name: String,
}

impl StdFileResource {
pub fn stdio(std_file: &StdFile, name: &str) -> Self {
Self {
fs_file: Some((
std_file.try_clone().map(|s| Arc::new(Mutex::new(s))).ok(),
Some(RefCell::new(FileMetadata::default())),
)),
fs_file: std_file.try_clone().map(|s| Arc::new(Mutex::new(s))).ok(),
metadata: RefCell::new(FileMetadata::default()),
name: name.to_string(),
..Default::default()
}
}

pub fn fs_file(fs_file: StdFile) -> Self {
Self {
fs_file: Some((
Some(Arc::new(Mutex::new(fs_file))),
Some(RefCell::new(FileMetadata::default())),
)),
fs_file: Some(Arc::new(Mutex::new(fs_file))),
metadata: RefCell::new(FileMetadata::default()),
name: "fsFile".to_string(),
..Default::default()
}
}

pub fn std_file(&self) -> Result<Arc<Mutex<StdFile>>, AnyError> {
match &self.fs_file {
Some(fs_file) => Ok(fs_file.clone()),
None => Err(bad_resource_id()),
}
}

pub fn metadata_mut(&self) -> std::cell::RefMut<FileMetadata> {
self.metadata.borrow_mut()
}

async fn read(
self: Rc<Self>,
mut buf: ZeroCopyBuf,
) -> Result<(usize, ZeroCopyBuf), AnyError> {
if self.fs_file.is_some() {
let fs_file = self.fs_file.as_ref().unwrap();
let std_file = fs_file.0.as_ref().unwrap().clone();
tokio::task::spawn_blocking(
move || -> Result<(usize, ZeroCopyBuf), AnyError> {
let mut std_file = std_file.lock().unwrap();
Ok((std_file.read(&mut buf)?, buf))
},
)
.await?
} else {
Err(resource_unavailable())
}
let std_file = self.fs_file.as_ref().unwrap().clone();
tokio::task::spawn_blocking(
move || -> Result<(usize, ZeroCopyBuf), AnyError> {
let mut std_file = std_file.lock().unwrap();
Ok((std_file.read(&mut buf)?, buf))
},
)
.await?
}

async fn write(self: Rc<Self>, buf: ZeroCopyBuf) -> Result<usize, AnyError> {
if self.fs_file.is_some() {
let fs_file = self.fs_file.as_ref().unwrap();
let std_file = fs_file.0.as_ref().unwrap().clone();
tokio::task::spawn_blocking(move || {
let mut std_file = std_file.lock().unwrap();
std_file.write(&buf)
})
.await?
.map_err(AnyError::from)
} else {
Err(resource_unavailable())
}
let std_file = self.fs_file.as_ref().unwrap().clone();
tokio::task::spawn_blocking(move || {
let mut std_file = std_file.lock().unwrap();
std_file.write(&buf)
})
.await?
.map_err(AnyError::from)
}

pub fn with<F, R>(
Expand All @@ -376,15 +372,8 @@ impl StdFileResource {
F: FnMut(Result<&mut std::fs::File, ()>) -> Result<R, AnyError>,
{
let resource = state.resource_table.get::<StdFileResource>(rid)?;
// TODO(@AaronO): does this make sense ?
// Sync write only works for FsFile. It doesn't make sense to do this
// for non-blocking sockets. So we error out if not FsFile.
if resource.fs_file.is_none() {
return f(Err(()));
}

let (r, _) = resource.fs_file.as_ref().unwrap();
match r {
match &resource.fs_file {
Some(r) => f(Ok(&mut r.as_ref().lock().unwrap())),
None => Err(resource_unavailable()),
}
Expand Down
39 changes: 8 additions & 31 deletions runtime/ops/tty.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

use super::io::StdFileResource;
use deno_core::error::bad_resource_id;
use deno_core::error::not_supported;
use deno_core::error::resource_unavailable;
use deno_core::error::AnyError;
use deno_core::op;
use deno_core::Extension;
Expand Down Expand Up @@ -88,23 +86,12 @@ fn op_set_raw(state: &mut OpState, args: SetRawArgs) -> Result<(), AnyError> {
let resource = state.resource_table.get::<StdFileResource>(rid)?;

if cbreak {
return Err(not_supported());
return Err(deno_core::error::not_supported());
}

if resource.fs_file.is_none() {
return Err(bad_resource_id());
}

let handle_result = if let Some((fs_file, _)) = &resource.fs_file {
let file = fs_file.as_ref().unwrap().clone();
let std_file = file.lock().unwrap();
let raw_handle = std_file.as_raw_handle();
Ok(raw_handle)
} else {
Err(resource_unavailable())
};

let handle = handle_result?;
let std_file = resource.std_file()?;
let std_file = std_file.lock().unwrap(); // hold the lock
let handle = std_file.as_raw_handle();

if handle == handleapi::INVALID_HANDLE_VALUE {
return Err(Error::last_os_error().into());
Expand Down Expand Up @@ -133,20 +120,10 @@ fn op_set_raw(state: &mut OpState, args: SetRawArgs) -> Result<(), AnyError> {
use std::os::unix::io::AsRawFd;

let resource = state.resource_table.get::<StdFileResource>(rid)?;

if resource.fs_file.is_none() {
return Err(not_supported());
}

let (fs_file, meta) =
resource.fs_file.as_ref().ok_or_else(resource_unavailable)?;

if fs_file.is_none() {
return Err(resource_unavailable());
}

let raw_fd = fs_file.as_ref().unwrap().lock().unwrap().as_raw_fd();
let maybe_tty_mode = &mut meta.as_ref().unwrap().borrow_mut().tty.mode;
let std_file = resource.std_file()?;
let raw_fd = std_file.lock().unwrap().as_raw_fd();
let mut meta_data = resource.metadata_mut();
let maybe_tty_mode = &mut meta_data.tty.mode;

if is_raw {
if maybe_tty_mode.is_none() {
Expand Down

0 comments on commit 11c13fb

Please sign in to comment.