Change implementation to support (and use) dyn Future for erased BoxScope

src/box_scope.rs

@@ -1,47 +1,44 @@
-use std::{alloc::Layout, cell::UnsafeCell, future::Future, ptr::NonNull};
-
-use crate::{
-    nofuture::{NoFuture, RawFuture},
-    raw_scope::RawScope,
-    Family, Never, TopScope,
+use std::{
+    future::Future,
+    mem::{self, MaybeUninit},
+    ptr::NonNull,
 };
 
+use crate::{raw_scope::RawScope, Family, Never, TopScope};
+
 /// A dynamic scope tied to a Box.
 ///
 /// This kind of scopes uses a dynamic allocation.
 /// In exchange, it is fully `'static` and can be moved after creation.
 #[repr(transparent)]
-pub struct BoxScope<T, F = NoFuture>(std::ptr::NonNull<RawScope<T, F>>)
+pub struct BoxScope<T, F: ?Sized = dyn Future<Output = Never> + 'static>(
+    std::ptr::NonNull<RawScope<T, F>>,
+)
 where
     T: for<'a> Family<'a>,
-    F: RawFuture<Output = Never>;
+    F: Future<Output = Never>;
 
-impl<T, F> Drop for BoxScope<T, F>
+impl<T, F: ?Sized> Drop for BoxScope<T, F>
 where
     T: for<'a> Family<'a>,
-    F: RawFuture<Output = Never>,
+    F: Future<Output = Never>,
 {
     fn drop(&mut self) {
-        // SAFETY: created from a Box in the constructor, so dereference-able.
-        let this = self.0.as_ptr();
-        // SAFETY: we MUST release the future before calling drop on the `Box` otherwise we'll call its
-        // destructor after releasing its backing memory, causing uaf
-        {
-            let fut = unsafe { std::ptr::addr_of!((*this).active_fut) };
-            let fut = UnsafeCell::raw_get(fut);
-            let fut: *mut F = fut.cast();
-            let fut = NonNull::new(fut).unwrap();
-            // SAFETY: a call to `RawScope::open` happened
-            unsafe {
-                RawFuture::drop_future(fut);
-                let this = self.0.as_ptr();
-                RawFuture::dealloc_outer(fut, this);
-            }
-        }
+        // SAFETY: this `Box::from_raw` pairs with a `Box::into_raw`
+        // in the `new_typed` constructor. The type `F` is not the same,
+        // but `MaybeUninit<F>` and `F` are repr(transparent)-compatible
+        // and RawScope is repr(C), so the Box frees the same memory.
+        // Furthermore, the `new_typed` constructor ensured that F is properly
+        // initialized so it may be dropped.
+        //
+        // Finally, the drop order of implicitly first dropping self.0.state
+        // and THEN self.0.active_fut goes a bit against the typical self-referencing
+        // structs assumptions, however self.0.state is a pointer and has no drop glue.
+        drop(unsafe { Box::from_raw(self.0.as_ptr()) })
     }
 }
 
-impl<T> BoxScope<T, NoFuture>
+impl<T> BoxScope<T>
 where
     T: for<'a> Family<'a>,
 {
@@ -55,24 +52,12 @@ where
     /// # Panics
     ///
     /// - If `scope` panics.
-    pub fn new_erased<S: TopScope<Family = T>>(scope: S) -> BoxScope<T, NoFuture>
+    pub fn new_erased<S: TopScope<Family = T>>(scope: S) -> Self
     where
         S::Future: 'static,
     {
-        let outer_layout = Layout::new::<RawScope<T, NoFuture<S::Future>>>();
-        let raw_scope = NonNull::new(
-            unsafe { std::alloc::alloc(outer_layout) } as *mut RawScope<T, NoFuture<S::Future>>
-        )
-        .unwrap();
-
-        unsafe { raw_scope.as_ptr().write(RawScope::new()) };
-
-        // SAFETY: `self.0` is dereference-able due to coming from a `Box`.
-        unsafe { RawScope::open_erased(raw_scope, outer_layout, scope) }
-
-        // SAFETY: open was called as part of `BoxScope::new`
-        let erased_raw_scope = unsafe { RawScope::erase(raw_scope) };
-        BoxScope(erased_raw_scope)
+        let this = mem::ManuallyDrop::new(BoxScope::new_typed(scope));
+        Self(this.0)
     }
 }
 
@@ -93,20 +78,46 @@ where
     where
         S: TopScope<Family = T>,
     {
-        let raw_scope = Box::new(RawScope::new());
-        let raw_scope = Box::leak(raw_scope).into();
+        let raw_scope = Box::new(RawScope::<T, F>::new_uninit());
+        let raw_scope: *mut RawScope<T, MaybeUninit<F>> = Box::into_raw(raw_scope);
+        struct Guard<Sc> {
+            raw_scope: *mut Sc,
+        }
+        // guard ensures Box is freed on panic (i.e. if scope.run panics)
+        let panic_guard = Guard { raw_scope };
+        impl<Sc> Drop for Guard<Sc> {
+            fn drop(&mut self) {
+                // SAFETY: defuse below makes sure this only happens on panic,
+                // in this case, self.raw_scope is still in the same uninitialized state
+                // and not otherwise being cleaned up, so this `Box::from_raw` pairs with
+                // `Box::into_raw` above
+                drop(unsafe { Box::from_raw(self.raw_scope) })
+            }
+        }
+
+        let raw_scope: *mut RawScope<T, F> = raw_scope.cast();
 
-        // SAFETY: `self.0` is dereference-able due to coming from a `Box`.
-        unsafe { RawScope::open(raw_scope, scope) }
+        // SAFETY:
+        // 1. `raw_scope` allocated by the `Box` so is valid memory, although the future is not yet initialized
+        // 2. `raw_scope` was created from a valid `RawScope::<T, MaybeUninit<F>>`, so `state` is fully initialized.
+        //
+        // Note: as a post-condition of `RawScope`, `raw_scope` is fully initialized.
+        unsafe {
+            RawScope::open(raw_scope, scope);
+        }
+
+        mem::forget(panic_guard); // defuse guard
+                                  // (guard field has no drop glue, so this does not leak anything, it just skips the above `Drop` impl)
 
-        BoxScope(raw_scope)
+        // SAFETY: `raw_scope` allocated by the `Box` so is non-null.
+        BoxScope(unsafe { NonNull::new_unchecked(raw_scope) })
     }
 }
 
-impl<T, F> BoxScope<T, F>
+impl<T, F: ?Sized> BoxScope<T, F>
 where
     T: for<'a> Family<'a>,
-    F: RawFuture<Output = Never>,
+    F: Future<Output = Never>,
 {
     /// Enters the scope, making it possible to access the data frozen inside of the scope.
     ///
@@ -119,7 +130,10 @@ where
     where
         G: for<'a> FnOnce(&'a mut <T as Family<'a>>::Family) -> Output,
     {
-        // SAFETY: `self.0` is dereference-able due to coming from a `Box`.
+        // SAFETY:
+        // 1. `self.0` is valid as a post-condition of `new_typed`.
+        // 2. The object pointed to by `self.0` did not move and won't before deallocation.
+        // 3. `BoxScope::enter` takes an exclusive reference and the reference passed to `f` cannot escape `f`.
         unsafe { RawScope::enter(self.0, f) }
     }
 }

src/lib.rs

@@ -1,5 +1,7 @@
 #![warn(rustdoc::broken_intra_doc_links)]
 #![warn(missing_docs)]
+#![deny(elided_lifetimes_in_paths)]
+#![deny(unsafe_op_in_unsafe_fn)]
 #![doc = include_str!("../README.md")]
 #![doc(
     html_favicon_url = "https://raw.githubusercontent.com/dureuill/nolife/main/assets/nolife-tr.png?raw=true"
@@ -11,7 +13,6 @@
 mod box_scope;
 #[cfg(not(miri))]
 pub mod counterexamples;
-mod nofuture;
 mod raw_scope;
 pub mod scope;
 #[doc(hidden)]