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

Add ResourceLimiterAsync, which can yield until resource is available #3393

Merged
merged 29 commits into from
Oct 25, 2021
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
18a355e
give sychronous ResourceLimiter an async alternative
Sep 28, 2021
147c8f8
rename
Oct 20, 2021
9d1b246
fix
Oct 20, 2021
67a6c27
pooling needs the store earlier
Oct 20, 2021
adf7521
introduce a failing test
Oct 21, 2021
2225722
Memory::new_async, grow_async now work!
Oct 21, 2021
a5007f3
runtime: use anyhow::Error instead of Box<dyn std::error::Error...>
Oct 21, 2021
abbe28d
propogate changes to use anyhow::Error instead of Box<dyn Error...>
Oct 21, 2021
6c70b81
review feedback
Oct 21, 2021
d3deaae
collapse some common code
Oct 21, 2021
5aef8f4
catch panic in libcalls for memory and table grow
Oct 21, 2021
252ba39
implement table _async methods, test passes now
Oct 21, 2021
351a51c
docs
Oct 21, 2021
c0a1af9
fix trap behavior
Oct 21, 2021
a1301f8
add table_grow_failed
Oct 21, 2021
538c195
build out async versions of the existing limiter tests
Oct 21, 2021
758abe3
add table limiting tests
Oct 21, 2021
3fd674c
async memory_grow_failed can have a default impl
Oct 21, 2021
175e72b
test that the catch unwind works
Oct 21, 2021
0370d5c
code review suggestion
Oct 21, 2021
efef076
make uffd test compile, but not pass
Oct 22, 2021
52542b6
mock enough of the store to pass the uffd test
Oct 22, 2021
b00d811
code review
Oct 22, 2021
bcbd746
add some tests that show behavior is the same when on wasm stack
Oct 22, 2021
9962897
docs
Oct 22, 2021
6819459
fix all docs links
Oct 22, 2021
5f978db
make feature requirement render in rustdoc for new apis
Oct 22, 2021
fb549c6
actually do some awaiting in the async limiter, which doesnt work
Oct 22, 2021
2f2c523
Add Alex's solution for null handling in TlsRestore
alexcrichton Oct 25, 2021
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
6 changes: 4 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ criterion = "0.3.4"
num_cpus = "1.13.0"
winapi = { version = "0.3.9", features = ['memoryapi'] }
memchr = "2.4"
async-trait = "0.1"

[build-dependencies]
anyhow = "1.0.19"
Expand Down
98 changes: 11 additions & 87 deletions crates/runtime/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,86 +33,6 @@ mod allocator;

pub use allocator::*;

/// Value returned by [`ResourceLimiter::instances`] default method
pub const DEFAULT_INSTANCE_LIMIT: usize = 10000;
/// Value returned by [`ResourceLimiter::tables`] default method
pub const DEFAULT_TABLE_LIMIT: usize = 10000;
/// Value returned by [`ResourceLimiter::memories`] default method
pub const DEFAULT_MEMORY_LIMIT: usize = 10000;

/// Used by hosts to limit resource consumption of instances.
///
/// An instance can be created with a resource limiter so that hosts can take into account
/// non-WebAssembly resource usage to determine if a linear memory or table should grow.
pub trait ResourceLimiter {
/// Notifies the resource limiter that an instance's linear memory has been
/// requested to grow.
///
/// * `current` is the current size of the linear memory in bytes.
/// * `desired` is the desired size of the linear memory in bytes.
/// * `maximum` is either the linear memory's maximum or a maximum from an
/// instance allocator, also in bytes. A value of `None`
/// indicates that the linear memory is unbounded.
///
/// This function should return `true` to indicate that the growing
/// operation is permitted or `false` if not permitted. Returning `true`
/// when a maximum has been exceeded will have no effect as the linear
/// memory will not grow.
///
/// This function is not guaranteed to be invoked for all requests to
/// `memory.grow`. Requests where the allocation requested size doesn't fit
/// in `usize` or exceeds the memory's listed maximum size may not invoke
/// this method.
fn memory_growing(&mut self, current: usize, desired: usize, maximum: Option<usize>) -> bool;

/// Notifies the resource limiter that growing a linear memory, permitted by
/// the `memory_growing` method, has failed.
///
/// Reasons for failure include: the growth exceeds the `maximum` passed to
/// `memory_growing`, or the operating system failed to allocate additional
/// memory. In that case, `error` might be downcastable to a `std::io::Error`.
fn memory_grow_failed(&mut self, _error: &Error) {}

/// Notifies the resource limiter that an instance's table has been requested to grow.
///
/// * `current` is the current number of elements in the table.
/// * `desired` is the desired number of elements in the table.
/// * `maximum` is either the table's maximum or a maximum from an instance allocator.
/// A value of `None` indicates that the table is unbounded.
///
/// This function should return `true` to indicate that the growing operation is permitted or
/// `false` if not permitted. Returning `true` when a maximum has been exceeded will have no
/// effect as the table will not grow.
fn table_growing(&mut self, current: u32, desired: u32, maximum: Option<u32>) -> bool;

/// The maximum number of instances that can be created for a `Store`.
///
/// Module instantiation will fail if this limit is exceeded.
///
/// This value defaults to 10,000.
fn instances(&self) -> usize {
DEFAULT_INSTANCE_LIMIT
}

/// The maximum number of tables that can be created for a `Store`.
///
/// Module instantiation will fail if this limit is exceeded.
///
/// This value defaults to 10,000.
fn tables(&self) -> usize {
DEFAULT_TABLE_LIMIT
}

/// The maximum number of linear memories that can be created for a `Store`
///
/// Instantiation will fail with an error if this limit is exceeded.
///
/// This value defaults to 10,000.
fn memories(&self) -> usize {
DEFAULT_MEMORY_LIMIT
}
}

/// A type that roughly corresponds to a WebAssembly instance, but is also used
/// for host-defined objects.
///
Expand Down Expand Up @@ -429,7 +349,11 @@ impl Instance {
/// Returns `None` if memory can't be grown by the specified amount
/// of pages. Returns `Some` with the old size in bytes if growth was
/// successful.
pub(crate) fn memory_grow(&mut self, index: MemoryIndex, delta: u64) -> Option<usize> {
pub(crate) fn memory_grow(
&mut self,
index: MemoryIndex,
delta: u64,
) -> Result<Option<usize>, Error> {
let (idx, instance) = if let Some(idx) = self.module.defined_memory_index(index) {
(idx, self)
} else {
Expand All @@ -441,10 +365,10 @@ impl Instance {
(foreign_memory_index, foreign_instance)
}
};
let limiter = unsafe { (*instance.store()).limiter() };
let store = unsafe { &mut *instance.store() };
let memory = &mut instance.memories[idx];

let result = unsafe { memory.grow(delta, limiter) };
let result = unsafe { memory.grow(delta, store) };
let vmmemory = memory.vmmemory();

// Update the state used by wasm code in case the base pointer and/or
Expand All @@ -468,7 +392,7 @@ impl Instance {
table_index: TableIndex,
delta: u32,
init_value: TableElement,
) -> Option<u32> {
) -> Result<Option<u32>, Error> {
let (defined_table_index, instance) =
self.get_defined_table_index_and_instance(table_index);
instance.defined_table_grow(defined_table_index, delta, init_value)
Expand All @@ -479,14 +403,14 @@ impl Instance {
table_index: DefinedTableIndex,
delta: u32,
init_value: TableElement,
) -> Option<u32> {
let limiter = unsafe { (*self.store()).limiter() };
) -> Result<Option<u32>, Error> {
let store = unsafe { &mut *self.store() };
let table = self
.tables
.get_mut(table_index)
.unwrap_or_else(|| panic!("no table for index {}", table_index.index()));

let result = unsafe { table.grow(delta, init_value, limiter) };
let result = unsafe { table.grow(delta, init_value, store) };

// Keep the `VMContext` pointers used by compiled Wasm code up to
// date.
Expand Down
73 changes: 49 additions & 24 deletions crates/runtime/src/instance/allocator.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::imports::Imports;
use crate::instance::{Instance, InstanceHandle, ResourceLimiter, RuntimeMemoryCreator};
use crate::instance::{Instance, InstanceHandle, RuntimeMemoryCreator};
use crate::memory::{DefaultMemoryCreator, Memory};
use crate::table::Table;
use crate::traphandlers::Trap;
Expand Down Expand Up @@ -58,13 +58,13 @@ pub struct InstanceAllocationRequest<'a> {
/// are a bit of a lie. This is done purely so a store can learn about
/// itself when it gets called as a host function, and additionally so this
/// runtime can access internals as necessary (such as the
/// VMExternRefActivationsTable or the ResourceLimiter).
/// VMExternRefActivationsTable or the resource limiter methods).
///
/// Note that this ends up being a self-pointer to the instance when stored.
/// The reason is that the instance itself is then stored within the store.
/// We use a number of `PhantomPinned` declarations to indicate this to the
/// compiler. More info on this in `wasmtime/src/store.rs`
pub store: Option<*mut dyn Store>,
pub store: StorePtr,

/// A list of all wasm data that can be referenced by the module that
/// will be allocated. The `Module` given here has active/passive data
Expand All @@ -77,6 +77,35 @@ pub struct InstanceAllocationRequest<'a> {
pub wasm_data: *const [u8],
}

/// A pointer to a Store. This Option<*mut dyn Store> is wrapped in a struct
/// so that the function to create a &mut dyn Store is a method on a member of
/// InstanceAllocationRequest, rather than on a &mut InstanceAllocationRequest
/// itself, because several use-sites require a split mut borrow on the
/// InstanceAllocationRequest.
pub struct StorePtr(Option<*mut dyn Store>);
impl StorePtr {
/// A pointer to no Store.
pub fn empty() -> Self {
Self(None)
}
/// A pointer to a Store.
pub fn new(ptr: *mut dyn Store) -> Self {
Self(Some(ptr))
}
/// The raw contents of this struct
pub fn as_raw(&self) -> Option<*mut dyn Store> {
self.0.clone()
}
/// Use the StorePtr as a mut ref to the Store.
/// Safety: must not be used outside the original lifetime of the borrow.
pub(crate) unsafe fn get(&mut self) -> Option<&mut dyn Store> {
match self.0 {
Some(ptr) => Some(&mut *ptr),
None => None,
}
}
}

/// An link error while instantiating a module.
#[derive(Error, Debug)]
#[error("Link error: {0}")]
Expand Down Expand Up @@ -430,7 +459,7 @@ fn initialize_instance(
}

unsafe fn initialize_vmcontext(instance: &mut Instance, req: InstanceAllocationRequest) {
if let Some(store) = req.store {
if let Some(store) = req.store.as_raw() {
*instance.interrupts() = (*store).vminterrupts();
*instance.externref_activations_table() = (*store).externref_activations_table().0;
instance.set_store(store);
Expand Down Expand Up @@ -581,17 +610,6 @@ pub struct OnDemandInstanceAllocator {
stack_size: usize,
}

// rustc is quite strict with the lifetimes when dealing with mutable borrows,
// so this is a little helper to get a shorter lifetime on `Option<&mut T>`
fn borrow_limiter<'a>(
limiter: &'a mut Option<&mut dyn ResourceLimiter>,
) -> Option<&'a mut dyn ResourceLimiter> {
match limiter {
Some(limiter) => Some(&mut **limiter),
None => None,
}
}

impl OnDemandInstanceAllocator {
/// Creates a new on-demand instance allocator.
pub fn new(mem_creator: Option<Arc<dyn RuntimeMemoryCreator>>, stack_size: usize) -> Self {
Expand All @@ -605,15 +623,19 @@ impl OnDemandInstanceAllocator {

fn create_tables(
module: &Module,
mut limiter: Option<&mut dyn ResourceLimiter>,
store: &mut StorePtr,
) -> Result<PrimaryMap<DefinedTableIndex, Table>, InstantiationError> {
let num_imports = module.num_imported_tables;
let mut tables: PrimaryMap<DefinedTableIndex, _> =
PrimaryMap::with_capacity(module.table_plans.len() - num_imports);
for table in &module.table_plans.values().as_slice()[num_imports..] {
tables.push(
Table::new_dynamic(table, borrow_limiter(&mut limiter))
.map_err(InstantiationError::Resource)?,
Table::new_dynamic(table, unsafe {
store
.get()
.expect("if module has table plans, store is not empty")
})
.map_err(InstantiationError::Resource)?,
);
}
Ok(tables)
Expand All @@ -622,7 +644,7 @@ impl OnDemandInstanceAllocator {
fn create_memories(
&self,
module: &Module,
mut limiter: Option<&mut dyn ResourceLimiter>,
store: &mut StorePtr,
) -> Result<PrimaryMap<DefinedMemoryIndex, Memory>, InstantiationError> {
let creator = self
.mem_creator
Expand All @@ -633,8 +655,12 @@ impl OnDemandInstanceAllocator {
PrimaryMap::with_capacity(module.memory_plans.len() - num_imports);
for plan in &module.memory_plans.values().as_slice()[num_imports..] {
memories.push(
Memory::new_dynamic(plan, creator, borrow_limiter(&mut limiter))
.map_err(InstantiationError::Resource)?,
Memory::new_dynamic(plan, creator, unsafe {
store
.get()
.expect("if module has memory plans, store is not empty")
})
.map_err(InstantiationError::Resource)?,
);
}
Ok(memories)
Expand All @@ -656,9 +682,8 @@ unsafe impl InstanceAllocator for OnDemandInstanceAllocator {
&self,
mut req: InstanceAllocationRequest,
) -> Result<InstanceHandle, InstantiationError> {
let mut limiter = req.store.and_then(|s| (*s).limiter());
let memories = self.create_memories(&req.module, borrow_limiter(&mut limiter))?;
let tables = Self::create_tables(&req.module, borrow_limiter(&mut limiter))?;
let memories = self.create_memories(&req.module, &mut req.store)?;
let tables = Self::create_tables(&req.module, &mut req.store)?;

let host_state = std::mem::replace(&mut req.host_state, Box::new(()));

Expand Down
Loading