diff options
author | Bartek IwaĆczuk <biwanczuk@gmail.com> | 2020-12-16 17:14:12 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-12-16 17:14:12 +0100 |
commit | 6984b63f2f3c8d0819fe2dced8252a81f3400ae7 (patch) | |
tree | 5201bc962f913927409ae2770aca48ffa3aaaa34 /core | |
parent | 9fe26f8ca189ac81d9c20c454b9dbfa5e1011c3f (diff) |
refactor: rewrite ops to use ResourceTable2 (#8512)
This commit migrates all ops to use new resource table
and "AsyncRefCell".
Old implementation of resource table was completely
removed and all code referencing it was updated to use
new system.
Diffstat (limited to 'core')
-rw-r--r-- | core/async_cell.rs | 15 | ||||
-rw-r--r-- | core/examples/http_bench_bin_ops.rs | 12 | ||||
-rw-r--r-- | core/examples/http_bench_json_ops.rs | 12 | ||||
-rw-r--r-- | core/lib.rs | 6 | ||||
-rw-r--r-- | core/ops.rs | 15 | ||||
-rw-r--r-- | core/resources.rs | 253 | ||||
-rw-r--r-- | core/resources2.rs | 146 |
7 files changed, 158 insertions, 301 deletions
diff --git a/core/async_cell.rs b/core/async_cell.rs index d11b83932..cd6c51682 100644 --- a/core/async_cell.rs +++ b/core/async_cell.rs @@ -1,10 +1,14 @@ // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. +use std::any::type_name; use std::any::Any; use std::borrow::Borrow; use std::cell::Cell; use std::cell::UnsafeCell; use std::collections::VecDeque; +use std::fmt; +use std::fmt::Debug; +use std::fmt::Formatter; use std::ops::Deref; use std::rc::Rc; @@ -45,6 +49,17 @@ impl<T: 'static> AsyncRefCell<T> { pub fn as_ptr(&self) -> *mut T { self.value.get() } + + pub fn into_inner(self) -> T { + assert!(self.borrow_count.get().is_empty()); + self.value.into_inner() + } +} + +impl<T> Debug for AsyncRefCell<T> { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { + write!(f, "AsyncRefCell<{}>", type_name::<T>()) + } } impl<T: Default + 'static> Default for AsyncRefCell<T> { diff --git a/core/examples/http_bench_bin_ops.rs b/core/examples/http_bench_bin_ops.rs index 1d7a76c3d..32529652b 100644 --- a/core/examples/http_bench_bin_ops.rs +++ b/core/examples/http_bench_bin_ops.rs @@ -170,7 +170,7 @@ fn op_listen( let std_listener = std::net::TcpListener::bind(&addr)?; std_listener.set_nonblocking(true)?; let listener = TcpListener::try_from(std_listener)?; - let rid = state.resource_table_2.add(listener); + let rid = state.resource_table.add(listener); Ok(rid) } @@ -181,7 +181,7 @@ fn op_close( ) -> Result<u32, Error> { debug!("close rid={}", rid); state - .resource_table_2 + .resource_table .close(rid) .map(|_| 0) .ok_or_else(bad_resource_id) @@ -196,11 +196,11 @@ async fn op_accept( let listener = state .borrow() - .resource_table_2 + .resource_table .get::<TcpListener>(rid) .ok_or_else(bad_resource_id)?; let stream = listener.accept().await?; - let rid = state.borrow_mut().resource_table_2.add(stream); + let rid = state.borrow_mut().resource_table.add(stream); Ok(rid) } @@ -214,7 +214,7 @@ async fn op_read( let stream = state .borrow() - .resource_table_2 + .resource_table .get::<TcpStream>(rid) .ok_or_else(bad_resource_id)?; stream.read(&mut bufs[0]).await @@ -230,7 +230,7 @@ async fn op_write( let stream = state .borrow() - .resource_table_2 + .resource_table .get::<TcpStream>(rid) .ok_or_else(bad_resource_id)?; stream.write(&bufs[0]).await diff --git a/core/examples/http_bench_json_ops.rs b/core/examples/http_bench_json_ops.rs index c4fcd6363..8cf4061cc 100644 --- a/core/examples/http_bench_json_ops.rs +++ b/core/examples/http_bench_json_ops.rs @@ -134,7 +134,7 @@ fn op_listen( let std_listener = std::net::TcpListener::bind(&addr)?; std_listener.set_nonblocking(true)?; let listener = TcpListener::try_from(std_listener)?; - let rid = state.resource_table_2.add(listener); + let rid = state.resource_table.add(listener); Ok(serde_json::json!({ "rid": rid })) } @@ -152,7 +152,7 @@ fn op_close( .unwrap(); debug!("close rid={}", rid); state - .resource_table_2 + .resource_table .close(rid) .map(|_| serde_json::json!(())) .ok_or_else(bad_resource_id) @@ -174,11 +174,11 @@ async fn op_accept( let listener = state .borrow() - .resource_table_2 + .resource_table .get::<TcpListener>(rid) .ok_or_else(bad_resource_id)?; let stream = listener.accept().await?; - let rid = state.borrow_mut().resource_table_2.add(stream); + let rid = state.borrow_mut().resource_table.add(stream); Ok(serde_json::json!({ "rid": rid })) } @@ -199,7 +199,7 @@ async fn op_read( let stream = state .borrow() - .resource_table_2 + .resource_table .get::<TcpStream>(rid) .ok_or_else(bad_resource_id)?; let nread = stream.read(&mut bufs[0]).await?; @@ -223,7 +223,7 @@ async fn op_write( let stream = state .borrow() - .resource_table_2 + .resource_table .get::<TcpStream>(rid) .ok_or_else(bad_resource_id)?; let nwritten = stream.write(&bufs[0]).await?; diff --git a/core/lib.rs b/core/lib.rs index 5846ad99d..6cecd4d75 100644 --- a/core/lib.rs +++ b/core/lib.rs @@ -17,7 +17,6 @@ mod normalize_path; mod ops; pub mod plugin_api; mod resources; -mod resources2; mod runtime; mod shared_queue; mod zero_copy_buf; @@ -64,10 +63,9 @@ pub use crate::ops::OpFn; pub use crate::ops::OpId; pub use crate::ops::OpState; pub use crate::ops::OpTable; +pub use crate::resources::Resource; +pub use crate::resources::ResourceId; pub use crate::resources::ResourceTable; -pub use crate::resources2::Resource; -pub use crate::resources2::ResourceId; -pub use crate::resources2::ResourceTable2; pub use crate::runtime::GetErrorClassFn; pub use crate::runtime::JsErrorCreateFn; pub use crate::runtime::JsRuntime; diff --git a/core/ops.rs b/core/ops.rs index bf10d3d86..2907d2552 100644 --- a/core/ops.rs +++ b/core/ops.rs @@ -4,6 +4,8 @@ use crate::error::bad_resource_id; use crate::error::type_error; use crate::error::AnyError; use crate::gotham_state::GothamState; +use crate::resources::ResourceTable; +use crate::runtime::GetErrorClassFn; use crate::BufVec; use crate::ZeroCopyBuf; use futures::Future; @@ -33,10 +35,9 @@ pub enum Op { /// Maintains the resources and ops inside a JS runtime. pub struct OpState { - pub resource_table: crate::ResourceTable, - pub resource_table_2: crate::resources2::ResourceTable, + pub resource_table: ResourceTable, pub op_table: OpTable, - pub get_error_class_fn: crate::runtime::GetErrorClassFn, + pub get_error_class_fn: GetErrorClassFn, gotham_state: GothamState, } @@ -47,7 +48,6 @@ impl Default for OpState { fn default() -> OpState { OpState { resource_table: Default::default(), - resource_table_2: Default::default(), op_table: OpTable::default(), get_error_class_fn: &|_| "Error", gotham_state: Default::default(), @@ -279,7 +279,11 @@ pub fn op_resources( _args: Value, _zero_copy: &mut [ZeroCopyBuf], ) -> Result<Value, AnyError> { - let serialized_resources = state.resource_table.entries(); + let serialized_resources: HashMap<u32, String> = state + .resource_table + .names() + .map(|(rid, name)| (rid, name.to_string())) + .collect(); Ok(json!(serialized_resources)) } @@ -300,5 +304,6 @@ pub fn op_close( .resource_table .close(rid as u32) .ok_or_else(bad_resource_id)?; + Ok(json!({})) } diff --git a/core/resources.rs b/core/resources.rs index 753fa9713..da3b634fc 100644 --- a/core/resources.rs +++ b/core/resources.rs @@ -1,20 +1,63 @@ // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. -// Think of Resources as File Descriptors. They are integers that are allocated by -// the privileged side of Deno to refer to various rust objects that need to be -// referenced between multiple ops. For example, network sockets are resources. -// Resources may or may not correspond to a real operating system file -// descriptor (hence the different name). +// Think of Resources as File Descriptors. They are integers that are allocated +// by the privileged side of Deno which refer to various rust objects that need +// to be persisted between various ops. For example, network sockets are +// resources. Resources may or may not correspond to a real operating system +// file descriptor (hence the different name). -use crate::resources2::ResourceId; +use std::any::type_name; use std::any::Any; +use std::any::TypeId; +use std::borrow::Cow; use std::collections::HashMap; +use std::iter::Iterator; +use std::rc::Rc; + +/// All objects that can be store in the resource table should implement the +/// `Resource` trait. +pub trait Resource: Any + 'static { + /// Returns a string representation of the resource which is made available + /// to JavaScript code through `op_resources`. The default implementation + /// returns the Rust type name, but specific resource types may override this + /// trait method. + fn name(&self) -> Cow<str> { + type_name::<Self>().into() + } + + /// Resources may implement the `close()` trait method if they need to do + /// resource specific clean-ups, such as cancelling pending futures, after a + /// resource has been removed from the resource table. + fn close(self: Rc<Self>) {} +} + +impl dyn Resource { + #[inline(always)] + fn is<T: Resource>(&self) -> bool { + self.type_id() == TypeId::of::<T>() + } -/// These store Deno's file descriptors. These are not necessarily the operating -/// system ones. -type ResourceMap = HashMap<ResourceId, (String, Box<dyn Any>)>; + #[inline(always)] + #[allow(clippy::needless_lifetimes)] + pub fn downcast_rc<'a, T: Resource>(self: &'a Rc<Self>) -> Option<&'a Rc<T>> { + if self.is::<T>() { + let ptr = self as *const Rc<_> as *const Rc<T>; + Some(unsafe { &*ptr }) + } else { + None + } + } +} -/// Map-like data structure storing Deno's resources (equivalent to file descriptors). +/// A `ResourceId` is an integer value referencing a resource. It could be +/// considered to be the Deno equivalent of a `file descriptor` in POSIX like +/// operating systems. Elsewhere in the code base it is commonly abbreviated +/// to `rid`. +// TODO: use `u64` instead? +pub type ResourceId = u32; + +/// Map-like data structure storing Deno's resources (equivalent to file +/// descriptors). /// /// Provides basic methods for element access. A resource can be of any type. /// Different types of resources can be stored in the same map, and provided @@ -24,156 +67,98 @@ type ResourceMap = HashMap<ResourceId, (String, Box<dyn Any>)>; /// the key in the map. #[derive(Default)] pub struct ResourceTable { - map: ResourceMap, - next_id: u32, + index: HashMap<ResourceId, Rc<dyn Resource>>, + next_rid: ResourceId, } impl ResourceTable { - /// Checks if the given resource ID is contained. - pub fn has(&self, rid: ResourceId) -> bool { - self.map.contains_key(&rid) - } - - /// Returns a shared reference to a resource. + /// Inserts resource into the resource table, which takes ownership of it. /// - /// Returns `None`, if `rid` is not stored or has a type different from `T`. - pub fn get<T: Any>(&self, rid: ResourceId) -> Option<&T> { - let (_, resource) = self.map.get(&rid)?; - resource.downcast_ref::<T>() - } - - /// Returns a mutable reference to a resource. + /// The resource type is erased at runtime and must be statically known + /// when retrieving it through `get()`. /// - /// Returns `None`, if `rid` is not stored or has a type different from `T`. - pub fn get_mut<T: Any>(&mut self, rid: ResourceId) -> Option<&mut T> { - let (_, resource) = self.map.get_mut(&rid)?; - resource.downcast_mut::<T>() - } - - // TODO: resource id allocation should probably be randomized for security. - fn next_rid(&mut self) -> ResourceId { - let next_rid = self.next_id; - self.next_id += 1; - next_rid as ResourceId + /// Returns a unique resource ID, which acts as a key for this resource. + pub fn add<T: Resource>(&mut self, resource: T) -> ResourceId { + self.add_rc(Rc::new(resource)) } - /// Inserts a resource, taking ownership of it. + /// Inserts a `Rc`-wrapped resource into the resource table. /// /// The resource type is erased at runtime and must be statically known /// when retrieving it through `get()`. /// /// Returns a unique resource ID, which acts as a key for this resource. - pub fn add(&mut self, name: &str, resource: Box<dyn Any>) -> ResourceId { - let rid = self.next_rid(); - let r = self.map.insert(rid, (name.to_string(), resource)); - assert!(r.is_none()); + pub fn add_rc<T: Resource>(&mut self, resource: Rc<T>) -> ResourceId { + let resource = resource as Rc<dyn Resource>; + let rid = self.next_rid; + let removed_resource = self.index.insert(rid, resource); + assert!(removed_resource.is_none()); + self.next_rid += 1; rid } - /// Returns a map of resource IDs to names. - /// - /// The name is the one specified during `add()`. To access resources themselves, - /// use the `get()` or `get_mut()` functions. - pub fn entries(&self) -> HashMap<ResourceId, String> { - self - .map - .iter() - .map(|(key, (name, _resource))| (*key, name.clone())) - .collect() - } - - // close(2) is done by dropping the value. Therefore we just need to remove - // the resource from the resource table. - pub fn close(&mut self, rid: ResourceId) -> Option<()> { - self.map.remove(&rid).map(|(_name, _resource)| ()) - } - - /// Removes the resource identified by `rid` and returns it. - /// - /// When the provided `rid` is stored, the associated resource will be removed. - /// Otherwise, nothing happens and `None` is returned. - /// - /// If the type `T` matches the resource's type, the resource will be returned. - /// If the type mismatches, `None` is returned, but the resource is still removed. - pub fn remove<T: Any>(&mut self, rid: ResourceId) -> Option<Box<T>> { - if let Some((_name, resource)) = self.map.remove(&rid) { - let res = match resource.downcast::<T>() { - Ok(res) => Some(res), - Err(_e) => None, - }; - return res; - } - None - } -} - -#[cfg(test)] -mod tests { - use super::*; - - struct FakeResource { - not_empty: u128, - } - - impl FakeResource { - fn new(value: u128) -> FakeResource { - FakeResource { not_empty: value } - } + /// Returns true if any resource with the given `rid` exists. + pub fn has(&self, rid: ResourceId) -> bool { + self.index.contains_key(&rid) } - #[test] - fn test_create_resource_table_default() { - let table = ResourceTable::default(); - assert_eq!(table.map.len(), 0); + /// Returns a reference counted pointer to the resource of type `T` with the + /// given `rid`. If `rid` is not present or has a type different than `T`, + /// this function returns `None`. + pub fn get<T: Resource>(&self, rid: ResourceId) -> Option<Rc<T>> { + self + .index + .get(&rid) + .and_then(|rc| rc.downcast_rc::<T>()) + .map(Clone::clone) } - #[test] - fn test_add_to_resource_table_not_empty() { - let mut table = ResourceTable::default(); - table.add("fake1", Box::new(FakeResource::new(1))); - table.add("fake2", Box::new(FakeResource::new(2))); - assert_eq!(table.map.len(), 2); + pub fn get_any(&self, rid: ResourceId) -> Option<Rc<dyn Resource>> { + self.index.get(&rid).map(Clone::clone) } - #[test] - fn test_add_to_resource_table_are_contiguous() { - let mut table = ResourceTable::default(); - let rid1 = table.add("fake1", Box::new(FakeResource::new(1))); - let rid2 = table.add("fake2", Box::new(FakeResource::new(2))); - assert_eq!(rid1 + 1, rid2); + /// Removes a resource of type `T` from the resource table and returns it. + /// If a resource with the given `rid` exists but its type does not match `T`, + /// it is not removed from the resource table. Note that the resource's + /// `close()` method is *not* called. + pub fn take<T: Resource>(&mut self, rid: ResourceId) -> Option<Rc<T>> { + let resource = self.get::<T>(rid)?; + self.index.remove(&rid); + Some(resource) } - #[test] - fn test_get_from_resource_table_is_what_was_given() { - let mut table = ResourceTable::default(); - let rid = table.add("fake", Box::new(FakeResource::new(7))); - let resource = table.get::<FakeResource>(rid); - assert_eq!(resource.unwrap().not_empty, 7); + /// Removes a resource from the resource table and returns it. Note that the + /// resource's `close()` method is *not* called. + pub fn take_any(&mut self, rid: ResourceId) -> Option<Rc<dyn Resource>> { + self.index.remove(&rid) } - #[test] - fn test_remove_from_resource_table() { - let mut table = ResourceTable::default(); - let rid1 = table.add("fake1", Box::new(FakeResource::new(1))); - let rid2 = table.add("fake2", Box::new(FakeResource::new(2))); - assert_eq!(table.map.len(), 2); - table.close(rid1); - assert_eq!(table.map.len(), 1); - table.close(rid2); - assert_eq!(table.map.len(), 0); + /// Removes the resource with the given `rid` from the resource table. If the + /// only reference to this resource existed in the resource table, this will + /// cause the resource to be dropped. However, since resources are reference + /// counted, therefore pending ops are not automatically cancelled. A resource + /// may implement the `close()` method to perform clean-ups such as canceling + /// ops. + pub fn close(&mut self, rid: ResourceId) -> Option<()> { + self.index.remove(&rid).map(|resource| resource.close()) } - #[test] - fn test_take_from_resource_table() { - let mut table = ResourceTable::default(); - let rid1 = table.add("fake1", Box::new(FakeResource::new(1))); - let rid2 = table.add("fake2", Box::new(FakeResource::new(2))); - assert_eq!(table.map.len(), 2); - let res1 = table.remove::<FakeResource>(rid1); - assert_eq!(table.map.len(), 1); - assert!(res1.is_some()); - let res2 = table.remove::<FakeResource>(rid2); - assert_eq!(table.map.len(), 0); - assert!(res2.is_some()); + /// Returns an iterator that yields a `(id, name)` pair for every resource + /// that's currently in the resource table. This can be used for debugging + /// purposes or to implement the `op_resources` op. Note that the order in + /// which items appear is not specified. + /// + /// # Example + /// + /// ``` + /// # use deno_core::ResourceTable; + /// # let resource_table = ResourceTable::default(); + /// let resource_names = resource_table.names().collect::<Vec<_>>(); + /// ``` + pub fn names(&self) -> impl Iterator<Item = (ResourceId, Cow<str>)> { + self + .index + .iter() + .map(|(&id, resource)| (id, resource.name())) } } diff --git a/core/resources2.rs b/core/resources2.rs deleted file mode 100644 index 989ea8328..000000000 --- a/core/resources2.rs +++ /dev/null @@ -1,146 +0,0 @@ -// Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. - -// Think of Resources as File Descriptors. They are integers that are allocated -// by the privileged side of Deno which refer to various rust objects that need -// to be persisted between various ops. For example, network sockets are -// resources. Resources may or may not correspond to a real operating system -// file descriptor (hence the different name). - -use std::any::type_name; -use std::any::Any; -use std::any::TypeId; -use std::borrow::Cow; -use std::collections::HashMap; -use std::iter::Iterator; -use std::rc::Rc; - -/// All objects that can be store in the resource table should implement the -/// `Resource` trait. -pub trait Resource: Any + 'static { - /// Returns a string representation of the resource which is made available - /// to JavaScript code through `op_resources`. The default implementation - /// returns the Rust type name, but specific resource types may override this - /// trait method. - fn name(&self) -> Cow<str> { - type_name::<Self>().into() - } - - /// Resources may implement the `close()` trait method if they need to do - /// resource specific clean-ups, such as cancelling pending futures, after a - /// resource has been removed from the resource table. - fn close(self: Rc<Self>) {} -} - -impl dyn Resource { - #[inline(always)] - fn is<T: Resource>(&self) -> bool { - self.type_id() == TypeId::of::<T>() - } - - #[inline(always)] - #[allow(clippy::needless_lifetimes)] - fn downcast_rc<'a, T: Resource>(self: &'a Rc<Self>) -> Option<&'a Rc<T>> { - if self.is::<T>() { - let ptr = self as *const Rc<_> as *const Rc<T>; - Some(unsafe { &*ptr }) - } else { - None - } - } -} - -/// A `ResourceId` is an integer value referencing a resource. It could be -/// considered to be the Deno equivalent of a `file descriptor` in POSIX like -/// operating systems. Elsewhere in the code base it is commonly abbreviated -/// to `rid`. -// TODO: use `u64` instead? -pub type ResourceId = u32; - -/// Temporary alias for `crate::resources2::ResourceTable`. -// TODO: remove this when the old `ResourceTable` is obsolete. -pub type ResourceTable2 = ResourceTable; - -/// Map-like data structure storing Deno's resources (equivalent to file -/// descriptors). -/// -/// Provides basic methods for element access. A resource can be of any type. -/// Different types of resources can be stored in the same map, and provided -/// with a name for description. -/// -/// Each resource is identified through a _resource ID (rid)_, which acts as -/// the key in the map. -#[derive(Default)] -pub struct ResourceTable { - index: HashMap<ResourceId, Rc<dyn Resource>>, - next_rid: ResourceId, -} - -impl ResourceTable { - /// Returns true if any resource with the given `rid` is exists. - pub fn has(&self, rid: ResourceId) -> bool { - self.index.contains_key(&rid) - } - - /// Returns a reference counted pointer to the resource of type `T` with the - /// given `rid`. If `rid` is not present or has a type different than `T`, - /// this function returns `None`. - pub fn get<T: Resource>(&self, rid: ResourceId) -> Option<Rc<T>> { - self - .index - .get(&rid) - .and_then(|resource| resource.downcast_rc::<T>()) - .map(Clone::clone) - } - - /// Inserts resource into the resource table, which takes ownership of it. - /// - /// The resource type is erased at runtime and must be statically known - /// when retrieving it through `get()`. - /// - /// Returns a unique resource ID, which acts as a key for this resource. - pub fn add<T: Resource>(&mut self, resource: T) -> ResourceId { - self.add_rc(Rc::new(resource)) - } - - /// Inserts a `Rc`-wrapped resource into the resource table. - /// - /// The resource type is erased at runtime and must be statically known - /// when retrieving it through `get()`. - /// - /// Returns a unique resource ID, which acts as a key for this resource. - pub fn add_rc<T: Resource>(&mut self, resource: Rc<T>) -> ResourceId { - let resource = resource as Rc<dyn Resource>; - let rid = self.next_rid; - let removed_resource = self.index.insert(rid, resource); - assert!(removed_resource.is_none()); - self.next_rid += 1; - rid - } - - /// Removes the resource with the given `rid` from the resource table. If the - /// only reference to this resource existed in the resource table, this will - /// cause the resource to be dropped. However, since resources are reference - /// counted, therefore pending ops are not automatically cancelled. - pub fn close(&mut self, rid: ResourceId) -> Option<()> { - self.index.remove(&rid).map(|resource| resource.close()) - } - - /// Returns an iterator that yields a `(id, name)` pair for every resource - /// that's currently in the resource table. This can be used for debugging - /// purposes or to implement the `op_resources` op. Note that the order in - /// which items appear is not specified. - /// - /// # Example - /// - /// ``` - /// # use deno_core::ResourceTable2; - /// # let resource_table = ResourceTable2::default(); - /// let resource_names = resource_table.names().collect::<Vec<_>>(); - /// ``` - pub fn names(&self) -> impl Iterator<Item = (ResourceId, Cow<str>)> { - self - .index - .iter() - .map(|(&id, resource)| (id, resource.name())) - } -} |