Add Send/Sync bounds to ComponentType

This commit is extracted from from review of bytecodealliance#11123 and bytecodealliance#11127. While
not literally present in those PRs it's my own personal conclusion that
it's best to just go ahead and add these bounds at the "base" of the
component trait hierarchy. The current implementation in bytecodealliance#11123 adds
bounds in many locations and this would remove the need to add bounds
everywhere and instead have everything inherited through the `Lift` and
`Lower` traits.

This raises the question of: why? The main conclusion that I've reached
leading to this change is that Wasmtime currently will store `R`, a
return value, on the stack during the lowering process back into linear
memory. This might involve allocation, however, meaning that wasm can be
invoked and a context switch could happen. For Wasmtime's `unsafe impl`
of `Send` and `Sync` on fibers to be sound it requires that this
stack-local variable is also `Send` and `Sync` as it's an entirely
user-provided type. Thus I've concluded that for results it's always
required for these to be both `Send` and `Sync` (or at the very least,
`Send`).

Given that I've gone ahead and updated to require both `Send` and `Sync`
for both params and results. This is not expected to actually have any
impact in practice since all primitives are already `Send`/`Sync` (minus
`Rc` impls all removed here) and all `bindgen!`-generated types are
compositions of `Send`/`Sync` primitives meaning that they're also
`Send` and `Sync`.

Author: alexcrichton

crates/wasmtime/src/runtime/component/func/typed.rs

@@ -678,7 +678,7 @@ pub unsafe trait ComponentNamedList: ComponentType {}
 /// [d-cm]: macro@crate::component::ComponentType
 /// [f-m]: crate::component::flags
 ///
-/// Rust standard library pointers such as `&T`, `Box<T>`, `Rc<T>`, and `Arc<T>`
+/// Rust standard library pointers such as `&T`, `Box<T>`, and `Arc<T>`
 /// additionally represent whatever type `T` represents in the component model.
 /// Note that types such as `record`, `variant`, `enum`, and `flags` are
 /// generated by the embedder at compile time. These macros derive
@@ -720,7 +720,29 @@ pub unsafe trait ComponentNamedList: ComponentType {}
 // Also note that this trait specifically is not sealed because we have a proc
 // macro that generates implementations of this trait for external types in a
 // `#[derive]`-like fashion.
-pub unsafe trait ComponentType {
+//
+// # Send and Sync
+//
+// While on the topic of safety it's worth discussing the `Send` and `Sync`
+// bounds here as well. These bounds might naively seem like they shouldn't be
+// required for all component types as they're host-level types not guest-level
+// types persisted anywhere. Various subtleties lead to these bounds, however:
+//
+// * Fibers require that all stack-local variables are `Send` and `Sync` for
+//   fibers themselves to be send/sync. Unfortunately we have no help from the
+//   compiler on this one so it's up to Wasmtime's discipline to maintain this.
+//   One instance of this is that return values are placed on the stack as
+//   they're lowered into guest memory. This lowering operation can involve
+//   malloc and context switches, so return values must be Send/Sync.
+//
+// * In the implementation of component model async it's not uncommon for types
+//   to be "buffered" in the store temporarily. For example parameters might
+//   reside in a store temporarily while wasm has backpressure turned on.
+//
+// Overall it's generally easiest to require `Send` and `Sync` for all
+// component types. There additionally aren't known use case for non-`Send` or
+// non-`Sync` types at this time.
+pub unsafe trait ComponentType: Send + Sync {
     /// Representation of the "lowered" form of this component value.
     ///
     /// Lowerings lower into core wasm values which are represented by `ValRaw`.
@@ -985,7 +1007,6 @@ macro_rules! forward_type_impls {
 forward_type_impls! {
     (T: ComponentType + ?Sized) &'_ T => T,
     (T: ComponentType + ?Sized) Box<T> => T,
-    (T: ComponentType + ?Sized) alloc::rc::Rc<T> => T,
     (T: ComponentType + ?Sized) alloc::sync::Arc<T> => T,
     () String => str,
     (T: ComponentType) Vec<T> => [T],
@@ -1018,7 +1039,6 @@ macro_rules! forward_lowers {
 forward_lowers! {
     (T: Lower + ?Sized) &'_ T => T,
     (T: Lower + ?Sized) Box<T> => T,
-    (T: Lower + ?Sized) alloc::rc::Rc<T> => T,
     (T: Lower + ?Sized) alloc::sync::Arc<T> => T,
     () String => str,
     (T: Lower) Vec<T> => [T],
@@ -1042,7 +1062,6 @@ macro_rules! forward_string_lifts {
 
 forward_string_lifts! {
     Box<str>,
-    alloc::rc::Rc<str>,
     alloc::sync::Arc<str>,
     String,
 }
@@ -1065,7 +1084,6 @@ macro_rules! forward_list_lifts {
 
 forward_list_lifts! {
     Box<[T]>,
-    alloc::rc::Rc<[T]>,
     alloc::sync::Arc<[T]>,
     Vec<T>,
 }

tests/all/component_model/func.rs

@@ -2,7 +2,6 @@
 
 use super::{REALLOC_AND_FREE, TypedFuncExt};
 use anyhow::Result;
-use std::rc::Rc;
 use std::sync::Arc;
 use wasmtime::component::*;
 use wasmtime::{Engine, Store, StoreContextMut, Trap};
@@ -426,15 +425,12 @@ fn type_layers() -> Result<()> {
     instance
         .get_typed_func::<(&u32,), ()>(&mut store, "take-u32")?
         .call_and_post_return(&mut store, (&2,))?;
-    instance
-        .get_typed_func::<(Rc<u32>,), ()>(&mut store, "take-u32")?
-        .call_and_post_return(&mut store, (Rc::new(2),))?;
     instance
         .get_typed_func::<(Arc<u32>,), ()>(&mut store, "take-u32")?
         .call_and_post_return(&mut store, (Arc::new(2),))?;
     instance
-        .get_typed_func::<(&Box<Arc<Rc<u32>>>,), ()>(&mut store, "take-u32")?
-        .call_and_post_return(&mut store, (&Box::new(Arc::new(Rc::new(2))),))?;
+        .get_typed_func::<(&Box<Arc<Box<u32>>>,), ()>(&mut store, "take-u32")?
+        .call_and_post_return(&mut store, (&Box::new(Arc::new(Box::new(2))),))?;
 
     Ok(())
 }