Review feedback.

crates/runtime/src/instance/allocator.rs

@@ -25,16 +25,6 @@ use wasmtime_environ::{
 #[cfg(feature = "pooling-allocator")]
 mod pooling;
 
-#[cfg(feature = "memfd-allocator")]
-mod memfd;
-#[cfg(feature = "memfd-allocator")]
-pub use self::memfd::MemFdSlot;
-
-#[cfg(not(feature = "memfd-allocator"))]
-mod memfd_disabled;
-#[cfg(not(feature = "memfd-allocator"))]
-pub use self::memfd_disabled::MemFdSlot;
-
 #[cfg(feature = "pooling-allocator")]
 pub use self::pooling::{
     InstanceLimits, ModuleLimits, PoolingAllocationStrategy, PoolingInstanceAllocator,

crates/runtime/src/instance/allocator/pooling.rs

@@ -7,11 +7,11 @@
 //! Using the pooling instance allocator can speed up module instantiation
 //! when modules can be constrained based on configurable limits.
 
-use super::MemFdSlot;
 use super::{
     initialize_instance, initialize_vmcontext, InstanceAllocationRequest, InstanceAllocator,
     InstanceHandle, InstantiationError,
 };
+use crate::MemFdSlot;
 use crate::{instance::Instance, Memory, Mmap, ModuleMemFds, Table};
 use anyhow::{anyhow, bail, Context, Result};
 use libc::c_void;
@@ -765,6 +765,22 @@ impl MemoryPool {
     }
 }
 
+impl Drop for MemoryPool {
+    fn drop(&mut self) {
+        // Clear the `clear_no_drop` flag (i.e., ask to *not* clear on
+        // drop) for all MemFdSlots, and then drop them here. This is
+        // valid because the one `Mmap` that covers the whole region
+        // can just do its one munmap.
+        for memfd in std::mem::take(&mut self.memfd_slots) {
+            if let Some(memfd_slot) = memfd.lock().unwrap().as_mut() {
+                unsafe {
+                    memfd_slot.no_clear_on_drop();
+                }
+            }
+        }
+    }
+}
+
 /// Represents a pool of WebAssembly tables.
 ///
 /// Each instance index into the pool returns an iterator over the base addresses

crates/runtime/src/lib.rs

@@ -69,45 +69,13 @@ pub use module_id::{CompiledModuleId, CompiledModuleIdAllocator};
 
 #[cfg(feature = "memfd-allocator")]
 mod memfd;
+#[cfg(feature = "memfd-allocator")]
+pub use crate::memfd::{MemFdSlot, MemoryMemFd, ModuleMemFds};
 
-pub use crate::memfd::MemoryMemFd;
-
-/// When memfd support is not included, provide a shim type and
-/// constructor instead so that higher-level code does not need
-/// feature-conditional compilation.
 #[cfg(not(feature = "memfd-allocator"))]
-#[allow(dead_code)]
-mod memfd {
-    use anyhow::Result;
-    use std::sync::Arc;
-    use wasmtime_environ::{DefinedMemoryIndex, Module};
-
-    /// A shim for the memfd image container when memfd support is not
-    /// included.
-    pub enum ModuleMemFds {}
-
-    /// A shim for an individual memory image.
-    #[allow(dead_code)]
-    pub enum MemoryMemFd {}
-
-    impl ModuleMemFds {
-        /// Construct a new set of memfd images. This variant is used
-        /// when memfd support is not included; it always returns no
-        /// images.
-        pub fn new(_: &Module, _: &[u8]) -> Result<Option<Arc<ModuleMemFds>>> {
-            Ok(None)
-        }
-
-        /// Get the memfd image for a particular memory.
-        pub(crate) fn get_memory_image(&self, _: DefinedMemoryIndex) -> Option<&Arc<MemoryMemFd>> {
-            // Should be unreachable because the `Self` type is
-            // uninhabitable.
-            match *self {}
-        }
-    }
-}
-
-pub use crate::memfd::ModuleMemFds;
+mod memfd_disabled;
+#[cfg(not(feature = "memfd-allocator"))]
+pub use crate::memfd_disabled::{MemFdSlot, MemoryMemFd, ModuleMemFds};
 
 /// Version number of this crate.
 pub const VERSION: &str = env!("CARGO_PKG_VERSION");