Fix a panic in the pooling allocator (#9549)

This panic is leftover from the development of the threads proposal so
move it to a first-class error instead of a runtime panic.

crates/wasmtime/src/runtime/vm/instance/allocator/pooling/memory_pool.rs

@@ -281,6 +281,17 @@ impl MemoryPool {
                     self.layout.max_memory_bytes,
                 );
             }
+            if memory.shared {
+                // FIXME(#4244): since the pooling allocator owns the memory
+                // allocation (which is torn down with the instance), that
+                // can't be used with shared memory where threads or the host
+                // might persist the memory beyond the lifetime of the instance
+                // itself.
+                bail!(
+                    "memory index {} is shared which is not supported in the pooling allocator",
+                    i.as_u32(),
+                );
+            }
         }
         Ok(())
     }

crates/wasmtime/src/runtime/vm/memory.rs

@@ -584,10 +584,9 @@ impl Memory {
         )?;
         let allocation = Box::new(pooled_memory);
         let allocation: Box<dyn RuntimeLinearMemory> = if ty.shared {
-            // FIXME: since the pooling allocator owns the memory allocation
-            // (which is torn down with the instance), the current shared memory
-            // implementation will cause problems; see
-            // https://github.com/bytecodealliance/wasmtime/issues/4244.
+            // FIXME(#4244): not supported with the pooling allocator (which
+            // `new_static` is always used with), see `MemoryPool::validate` as
+            // well).
             todo!("using shared memory with the pooling allocator is a work in progress");
         } else {
             allocation

docs/stability-wasm-proposals.md

@@ -23,7 +23,7 @@ column is below.
 | [`component-model`]      | ❌[^1]  | ✅    | ✅       | ⚠️[^2]  | ✅  | ❌[^5]|
 | [`relaxed-simd`]         | ✅      | ✅    | ✅       | ✅     | ✅  | ✅    |
 | [`multi-memory`]         | ✅      | ✅    | ✅       | ✅     | ✅  | ✅    |
-| [`threads`]              | ✅      | ✅    | ✅       | ❌[^3] | ✅  | ✅    |
+| [`threads`]              | ✅      | ✅    | ✅[^9]   | ❌[^3] | ✅  | ✅    |
 | [`tail-call`]            | ✅      | ✅    | ✅       | ✅     | ✅  | ✅    |
 | [`extended-const`]       | ✅      | ✅    | ✅       | ❌[^4] | ✅  | ✅    |
 
@@ -37,6 +37,11 @@ column is below.
     [`extended-const`] is not yet implemented in `wasm-smith`.
 [^5]: Support for the C API for components is desired by many embedders but
     does not currently have anyone lined up to implement it.
+[^9]: There are [known
+    issues](https://github.com/bytecodealliance/wasmtime/issues/4245) with
+    shared memories and the implementation/API in Wasmtime, for example they
+    aren't well integrated with resource-limiting features in `Store`.
+    Additionally `shared` memories aren't supported in the pooling allocator.
 
 ## Off-by-default proposals
 

tests/all/pooling_allocator.rs

@@ -1252,6 +1252,36 @@ fn tricky_empty_table_with_empty_virtual_memory_alloc() -> Result<()> {
     Ok(())
 }
 
+#[test]
+#[cfg_attr(miri, ignore)]
+fn shared_memory_unsupported() -> Result<()> {
+    let mut config = Config::new();
+    let mut cfg = PoolingAllocationConfig::default();
+    // shrink the size of this allocator
+    cfg.total_memories(1);
+    config.allocation_strategy(InstanceAllocationStrategy::Pooling(cfg));
+    let engine = Engine::new(&config)?;
+
+    let err = Module::new(
+        &engine,
+        r#"
+            (module
+                (memory 5 5 shared)
+            )
+        "#,
+    )
+    .unwrap_err();
+    let err = err.to_string();
+    assert!(
+        err.contains(
+            "memory index 0 is shared which is not supported \
+             in the pooling allocator"
+        ),
+        "bad error: {err}"
+    );
+    Ok(())
+}
+
 #[test]
 #[cfg_attr(miri, ignore)]
 fn custom_page_sizes_reusing_same_slot() -> Result<()> {