Returning a coroutine's stack to pool is safe

src/arch/x86_64.rs

@@ -73,19 +73,20 @@ pub unsafe extern "system" fn transfer_control(record: &mut Control) {
 // this is a macro rather than a function, because we need to call the `ret`
 // instruction in the current context. This is the only legal way to replace
 // the stack pointer value from within an inline assembly block in Rust.
-macro_rules! terminate {
-    ($control:expr) => {
+macro_rules! return_control {
+    ($record:expr) => {
         core::arch::asm!(
             // todo: empty the stack by replacing the stack pointer in the control record
             //       prior to returning. This effectively undoes the first set
             //       of instructions in `transfer_control`.
-            "mov rsp, [{new_sp_addr}]",
-            "ret",
-            new_sp_addr = in(reg) $control.stack_ptr,
-            //stack_ptr_offset = const offset_of!(Control, stack_ptr),
+            "mov rsp, [{ctrl_record} + {stack_ptr_offset}]",
+            "jmp [{ctrl_record} + {instr_ptr_offset}]",
+            ctrl_record = in(reg) $record,
+            stack_ptr_offset = const core::mem::offset_of!(Control, stack_ptr),
+            instr_ptr_offset = const core::mem::offset_of!(Control, instr_ptr),
             options(noreturn, nomem)
-        );
+        )
     };
 }
 
-pub(crate) use terminate;
+pub(crate) use return_control;

src/lib.rs

@@ -53,7 +53,7 @@ struct Control {
     /// was last suspended.
     ///
     /// This field is of use when [resuming] a coroutine that has not [finished]
-    /// its execution, because it begins where it last left off.
+    /// its execution, because it does so at the point where it last left off.
     ///
     /// [`trampoline` function]: Coro::new
     /// [resuming]: Coro::resume
@@ -193,15 +193,13 @@ impl<'f> Coro<'f> {
             };
             // Execute the provided closure in the stack of the coroutine.
             coro_fn();
-            // At this point the coroutine has finished its execution. Destroy
-            // the closure, restore the stack pointer to the value present in
-            // the control record, and return to the last caller of `resume`.
-            // Any attempt to resume the coroutine ever again will trigger a
-            // panic.
-            drop(coro_fn);
+            // At this point the coroutine has finished its execution. We need
+            // to restore the stack pointer to the value present in the control
+            // record, and return to the last caller of `resume`. Any attempt
+            // to resume the coroutine ever again will trigger a panic.
             // SAFETY: We have exclusive access to the stack.
             let control = unsafe { current_control() };
-            unsafe { arch::terminate!(control) }
+            unsafe { arch::return_control!(control) }
         }
         // Allocate a stack for the coroutine, and initialize its control record
         // as follows: First set the value of the stack pointer to the location
@@ -310,9 +308,10 @@ impl<'f> Coroutine for Coro<'f> {
     type Return = ();
 
     fn resume(self: Pin<&mut Self>, _arg: ()) -> CoroutineState<Self::Yield, Self::Return> {
+        // todo: document + add safety comments
         let coro = self.get_mut();
         {
-            let control = unsafe { coro.control_mut() };
+            let control = coro.control_mut();
             #[cfg(feature = "std")]
             eprintln!(
                 "[resuming] status: sp={:?}, rp={:?}",

src/stack.rs

@@ -321,5 +321,10 @@ impl StackPool {
             "cannot take stack of suspended coroutine"
         );
         self.0.push(coro.stack);
+        // Section 10.8 of [_The Rust Language Reference_ (4d292b6)] states
+        // that only the remaining fields are dropped after a partial move;
+        // this property prevents a double-free of the stack instance.
+        //
+        // [_The Rust Language Reference_ (4d292b6)]: https://doc.rust-lang.org/reference/
     }
 }