Skip to content

Commit bd98771

Browse files
authored
Fix current instance count on failure during component instantiation (#13048)
This commit fixes an issue during component instantiation where if instantiation failed during creation of the `Instantiator` due to OOM or if the `async` future was dropped the concurrent instance count in the pooling allocator would get corrupted (off-by-one). The fix here is to switch a `map_err` with a manual decrement to using a value that has a destructor to ensure that it handles both of these cases.
1 parent f2807a1 commit bd98771

3 files changed

Lines changed: 79 additions & 13 deletions

File tree

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
#![cfg(arc_try_new)]
2+
3+
use wasmtime::component::{Component, Linker};
4+
use wasmtime::{Config, Engine, PoolingAllocationConfig, Result, Store};
5+
use wasmtime_fuzzing::oom::OomTest;
6+
7+
#[test]
8+
fn instantiate() -> Result<()> {
9+
let mut config = Config::new();
10+
config.concurrency_support(false);
11+
let engine = Engine::new(&config)?;
12+
let component = Component::new(&engine, "(component)")?;
13+
let linker = Linker::<()>::new(&engine);
14+
let instance_pre = linker.instantiate_pre(&component)?;
15+
16+
OomTest::new().test(|| {
17+
let mut store = Store::try_new(&engine, ())?;
18+
let _instance = instance_pre.instantiate(&mut store)?;
19+
Ok(())
20+
})
21+
}
22+
23+
#[test]
24+
fn instantiate_in_pooling_allocator() -> Result<()> {
25+
let mut pool_config = PoolingAllocationConfig::default();
26+
pool_config.total_component_instances(1);
27+
28+
let mut config = Config::new();
29+
config.concurrency_support(false);
30+
config.allocation_strategy(pool_config);
31+
32+
let engine = Engine::new(&config)?;
33+
let component = Component::new(&engine, "(component)")?;
34+
let linker = Linker::<()>::new(&engine);
35+
let instance_pre = linker.instantiate_pre(&component)?;
36+
37+
OomTest::new().test(|| {
38+
let mut store = Store::try_new(&engine, ())?;
39+
let _instance = instance_pre.instantiate(&mut store)?;
40+
Ok(())
41+
})
42+
}

crates/fuzzing/tests/oom/main.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ mod boxed;
66
mod btree_map;
77
mod caller;
88
mod component_func;
9+
mod component_instance;
910
mod component_linker;
1011
mod component_resource;
1112
mod config;

crates/wasmtime/src/runtime/component/instance.rs

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1187,23 +1187,46 @@ impl<T: 'static> InstancePre<T> {
11871187
mut store: impl AsContextMut<Data = T>,
11881188
asyncness: Asyncness,
11891189
) -> Result<Instance> {
1190-
let mut store = store.as_context_mut();
1190+
let store = store.as_context_mut();
11911191
store.0.set_async_required(self.asyncness);
11921192
store
11931193
.engine()
11941194
.allocator()
11951195
.increment_component_instance_count()?;
1196-
let mut instantiator = Instantiator::new(&self.component, store.0, &self.imports)?;
1197-
instantiator.run(&mut store, asyncness).await.map_err(|e| {
1198-
store
1199-
.engine()
1200-
.allocator()
1201-
.decrement_component_instance_count();
1202-
e
1203-
})?;
1204-
1205-
let instance = Instance::from_wasmtime(store.0, instantiator.id);
1206-
store.0.push_component_instance(instance);
1207-
Ok(instance)
1196+
1197+
// Helper structure to pair the above increment with a decrement should
1198+
// anything fail below.
1199+
let mut decrement = DecrementComponentInstanceCountOnDrop {
1200+
store,
1201+
enabled: true,
1202+
};
1203+
1204+
let mut instantiator =
1205+
Instantiator::new(&self.component, decrement.store.0, &self.imports)?;
1206+
instantiator.run(&mut decrement.store, asyncness).await?;
1207+
1208+
let instance = Instance::from_wasmtime(decrement.store.0, instantiator.id);
1209+
decrement.store.0.push_component_instance(instance);
1210+
1211+
// Everything has passed, don't decrement the instance count and let the
1212+
// destructor for the `Store` handle that at this point.
1213+
decrement.enabled = false;
1214+
return Ok(instance);
1215+
1216+
struct DecrementComponentInstanceCountOnDrop<'a, T: 'static> {
1217+
store: StoreContextMut<'a, T>,
1218+
enabled: bool,
1219+
}
1220+
1221+
impl<T> Drop for DecrementComponentInstanceCountOnDrop<'_, T> {
1222+
fn drop(&mut self) {
1223+
if self.enabled {
1224+
self.store
1225+
.engine()
1226+
.allocator()
1227+
.decrement_component_instance_count();
1228+
}
1229+
}
1230+
}
12081231
}
12091232
}

0 commit comments

Comments
 (0)