Skip to content

Commit 5708666

Browse files
alexcrichtondicej
andauthored
Refactor how imported tasks and bindings work (#1260)
* Abstract `Waitable` and its code * Refactor how imported tasks and bindings work This commit refactors imported tasks to work the same way as futures/streams using a `WaitableOperation<S>` for managing all state associated with the task. This involved refactoring generating bindings and redoing how imported tasks are represented. This notably creates a "hole" for cancellation to slot into in the near future and additionally addresses a number of issues around leaking memory in async imports upon cancellation. Additionally to get tests passing in wasip3-prototyping it was necessary to fix an issue where a stack-local pointer was deallocated with a heap-deallocation function. Fixing this required refactoring how exported tasks worked which involved siloing abstractions from each other and restructuring the flow of information throughout `FutureState`. In a sense these are refactorings I wanted to get around to anyway, but they were required to be here as well to pass upstream tests. * Update crates/core/src/abi.rs Co-authored-by: Joel Dice <joel.dice@fermyon.com> --------- Co-authored-by: Joel Dice <joel.dice@fermyon.com>
1 parent dba6e3b commit 5708666

File tree

12 files changed

+1026
-711
lines changed

12 files changed

+1026
-711
lines changed

crates/core/src/abi.rs

Lines changed: 69 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -551,9 +551,6 @@ def_instruction! {
551551
blocks: usize,
552552
} : [1] => [0],
553553

554-
/// Call an async-lowered import.
555-
AsyncCallWasm { name: &'a str } : [2] => [0],
556-
557554
/// Generate code to run after `CallInterface` for an async-lifted export.
558555
///
559556
/// For example, this might include task management for the
@@ -831,6 +828,17 @@ fn needs_post_return(resolve: &Resolve, ty: &Type) -> bool {
831828
}
832829
}
833830

831+
/// Generate instructions in `bindgen` to deallocate all lists in `ptr` where
832+
/// that's a pointer to a sequence of `types` stored in linear memory.
833+
pub fn deallocate_lists_in_types<B: Bindgen>(
834+
resolve: &Resolve,
835+
types: &[Type],
836+
ptr: B::Operand,
837+
bindgen: &mut B,
838+
) {
839+
Generator::new(resolve, bindgen).deallocate_lists_in_types(types, ptr);
840+
}
841+
834842
#[derive(Copy, Clone)]
835843
pub enum Realloc {
836844
None,
@@ -877,6 +885,8 @@ impl<'a, B: Bindgen> Generator<'a, B> {
877885

878886
match lift_lower {
879887
LiftLower::LowerArgsLiftResults => {
888+
assert!(!async_, "generators should not be using this for async");
889+
880890
self.realloc = Some(realloc);
881891
if let (AbiVariant::GuestExport, true) = (variant, async_) {
882892
unimplemented!("host-side code generation for async lift/lower not supported");
@@ -894,84 +904,62 @@ impl<'a, B: Bindgen> Generator<'a, B> {
894904
self_.stack.push(ptr);
895905
};
896906

897-
if async_ {
907+
if !sig.indirect_params {
908+
// If the parameters for this function aren't indirect
909+
// (there aren't too many) then we simply do a normal lower
910+
// operation for them all.
911+
for (nth, (_, ty)) in func.params.iter().enumerate() {
912+
self.emit(&Instruction::GetArg { nth });
913+
self.lower(ty);
914+
}
915+
} else {
916+
// ... otherwise if parameters are indirect space is
917+
// allocated for them and each argument is lowered
918+
// individually into memory.
898919
let ElementInfo { size, align } = self
899920
.bindgen
900921
.sizes()
901-
.record(func.params.iter().map(|(_, ty)| ty));
902-
let ptr = self.bindgen.return_pointer(size, align);
903-
lower_to_memory(self, ptr);
904-
} else {
905-
if !sig.indirect_params {
906-
// If the parameters for this function aren't indirect
907-
// (there aren't too many) then we simply do a normal lower
908-
// operation for them all.
909-
for (nth, (_, ty)) in func.params.iter().enumerate() {
910-
self.emit(&Instruction::GetArg { nth });
911-
self.lower(ty);
922+
.record(func.params.iter().map(|t| &t.1));
923+
let ptr = match variant {
924+
// When a wasm module calls an import it will provide
925+
// space that isn't explicitly deallocated.
926+
AbiVariant::GuestImport => self.bindgen.return_pointer(size, align),
927+
// When calling a wasm module from the outside, though,
928+
// malloc needs to be called.
929+
AbiVariant::GuestExport => {
930+
self.emit(&Instruction::Malloc {
931+
realloc: "cabi_realloc",
932+
size,
933+
align,
934+
});
935+
self.stack.pop().unwrap()
912936
}
913-
} else {
914-
// ... otherwise if parameters are indirect space is
915-
// allocated from them and each argument is lowered
916-
// individually into memory.
917-
let ElementInfo { size, align } = self
918-
.bindgen
919-
.sizes()
920-
.record(func.params.iter().map(|t| &t.1));
921-
let ptr = match variant {
922-
// When a wasm module calls an import it will provide
923-
// space that isn't explicitly deallocated.
924-
AbiVariant::GuestImport => self.bindgen.return_pointer(size, align),
925-
// When calling a wasm module from the outside, though,
926-
// malloc needs to be called.
927-
AbiVariant::GuestExport => {
928-
self.emit(&Instruction::Malloc {
929-
realloc: "cabi_realloc",
930-
size,
931-
align,
932-
});
933-
self.stack.pop().unwrap()
934-
}
935-
AbiVariant::GuestImportAsync
936-
| AbiVariant::GuestExportAsync
937-
| AbiVariant::GuestExportAsyncStackful => {
938-
unreachable!()
939-
}
940-
};
941-
lower_to_memory(self, ptr);
942-
}
937+
AbiVariant::GuestImportAsync
938+
| AbiVariant::GuestExportAsync
939+
| AbiVariant::GuestExportAsyncStackful => {
940+
unreachable!()
941+
}
942+
};
943+
lower_to_memory(self, ptr);
943944
}
944945
self.realloc = None;
945946

946-
if async_ {
947-
let ElementInfo { size, align } =
948-
self.bindgen.sizes().record(func.result.iter());
949-
let ptr = self.bindgen.return_pointer(size, align);
947+
// If necessary we may need to prepare a return pointer for
948+
// this ABI.
949+
if variant == AbiVariant::GuestImport && sig.retptr {
950+
let info = self.bindgen.sizes().params(&func.result);
951+
let ptr = self.bindgen.return_pointer(info.size, info.align);
950952
self.return_pointer = Some(ptr.clone());
951953
self.stack.push(ptr);
952-
953-
assert_eq!(self.stack.len(), 2);
954-
self.emit(&Instruction::AsyncCallWasm {
955-
name: &format!("[async-lower]{}", func.name),
956-
});
957-
} else {
958-
// If necessary we may need to prepare a return pointer for
959-
// this ABI.
960-
if variant == AbiVariant::GuestImport && sig.retptr {
961-
let info = self.bindgen.sizes().params(&func.result);
962-
let ptr = self.bindgen.return_pointer(info.size, info.align);
963-
self.return_pointer = Some(ptr.clone());
964-
self.stack.push(ptr);
965-
}
966-
967-
assert_eq!(self.stack.len(), sig.params.len());
968-
self.emit(&Instruction::CallWasm {
969-
name: &func.name,
970-
sig: &sig,
971-
});
972954
}
973955

974-
if !(sig.retptr || async_) {
956+
assert_eq!(self.stack.len(), sig.params.len());
957+
self.emit(&Instruction::CallWasm {
958+
name: &func.name,
959+
sig: &sig,
960+
});
961+
962+
if !sig.retptr {
975963
// With no return pointer in use we can simply lift the
976964
// result(s) of the function from the result of the core
977965
// wasm function.
@@ -986,7 +974,7 @@ impl<'a, B: Bindgen> Generator<'a, B> {
986974
// `self.return_pointer`) so we use that to read
987975
// the result of the function from memory.
988976
AbiVariant::GuestImport => {
989-
assert!(sig.results.is_empty() || async_);
977+
assert!(sig.results.is_empty());
990978
self.return_pointer.take().unwrap()
991979
}
992980

@@ -1178,10 +1166,18 @@ impl<'a, B: Bindgen> Generator<'a, B> {
11781166

11791167
self.emit(&Instruction::GetArg { nth: 0 });
11801168
let addr = self.stack.pop().unwrap();
1181-
for (offset, ty) in self.bindgen.sizes().field_offsets(&func.result) {
1169+
1170+
let mut types = Vec::new();
1171+
types.extend(func.result);
1172+
self.deallocate_lists_in_types(&types, addr);
1173+
1174+
self.emit(&Instruction::Return { func, amt: 0 });
1175+
}
1176+
1177+
fn deallocate_lists_in_types(&mut self, types: &[Type], addr: B::Operand) {
1178+
for (offset, ty) in self.bindgen.sizes().field_offsets(types) {
11821179
self.deallocate(ty, addr.clone(), offset);
11831180
}
1184-
self.emit(&Instruction::Return { func, amt: 0 });
11851181

11861182
assert!(
11871183
self.stack.is_empty(),

crates/csharp/src/function.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1263,8 +1263,7 @@ impl Bindgen for FunctionBindgen<'_, '_> {
12631263
| Instruction::StreamLower { .. }
12641264
| Instruction::StreamLift { .. }
12651265
| Instruction::ErrorContextLower { .. }
1266-
| Instruction::ErrorContextLift { .. }
1267-
| Instruction::AsyncCallWasm { .. } => todo!(),
1266+
| Instruction::ErrorContextLift { .. } => todo!(),
12681267
}
12691268
}
12701269

0 commit comments

Comments
 (0)