use a synthetic coroutine frame for msvc destroy-n-continue workaround

include/stdexec/__detail/__connect_awaitable.hpp

@@ -71,21 +71,6 @@ namespace STDEXEC
       __with_await_transform() = default;
     };
 
-    struct __synthetic_coro_frame
-    {
-      void (*__resume_)(void*) noexcept;
-      // we never invoke __destroy_ so a no-op implementation is fine; we've chosen
-      // the address of a no-op function rather than nullptr in case some rogue awaitable
-      // *does* invoke destroy on the synthesized handle that it receives in its
-      // await_suspend function
-      void (*__destroy_)(void*) noexcept = &__noop_destroy;
-
-     private:
-      static void __noop_destroy(void*) noexcept {}
-    };
-
-    static constexpr std::ptrdiff_t __promise_offset = sizeof(__synthetic_coro_frame);
-
     template <class _Awaiter, class _Receiver>
     struct __opstate;
 
@@ -120,13 +105,13 @@ namespace STDEXEC
       __opstate_t& __get_opstate() noexcept
       {
         return *reinterpret_cast<__opstate_t*>(reinterpret_cast<std::byte*>(this)
-                                               - __promise_offset);
+                                               - __detail::__coro_promise_offset);
       }
 
       __opstate_t const & __get_opstate() const noexcept
       {
         return *reinterpret_cast<__opstate_t const *>(reinterpret_cast<std::byte const *>(this)
-                                                      - __promise_offset);
+                                                      - __detail::__coro_promise_offset);
       }
     };
 
@@ -500,7 +485,7 @@ namespace STDEXEC
         STDEXEC::set_stopped(static_cast<_Receiver&&>(__rcvr_));
       }
 
-      __synthetic_coro_frame                     __synthetic_frame_{&__promise_t::__resume};
+      __detail::__synthetic_coro_frame           __synthetic_frame_{&__promise_t::__resume};
       STDEXEC_IMMOVABLE_NO_UNIQUE_ADDRESS
       _Receiver                                  __rcvr_;
       STDEXEC_IMMOVABLE_NO_UNIQUE_ADDRESS

include/stdexec/coroutine.hpp

@@ -33,8 +33,7 @@ namespace STDEXEC
     return __std::coroutine_handle<_Tp>::from_address(__h.address());
   }
 
-  STDEXEC_ATTRIBUTE(always_inline)
-  void __coroutine_resume_nothrow(__std::coroutine_handle<> __h) noexcept  //
+  inline void __coroutine_resume_nothrow(__std::coroutine_handle<> __h) noexcept
   {
     STDEXEC_TRY
     {
@@ -48,6 +47,20 @@ namespace STDEXEC
     }
   }
 
+  inline void __coroutine_destroy_nothrow(__std::coroutine_handle<> __h) noexcept
+  {
+    STDEXEC_TRY
+    {
+      STDEXEC_ASSERT(__h);
+      __h.destroy();
+    }
+    STDEXEC_CATCH_ALL
+    {
+      STDEXEC_ASSERT(!"Coroutine destroy threw an exception!");
+      __std::unreachable();
+    }
+  }
+
   // A coroutine handle that also supports unhandled_stopped() for propagating stop
   // signals through co_awaits of senders.
   template <class _Promise = void>
@@ -140,118 +153,77 @@ namespace STDEXEC
     }
   };
 
-#  if STDEXEC_MSVC() && STDEXEC_MSVC_VERSION <= 1939
-  // MSVCBUG https://developercommunity.visualstudio.com/t/destroy-coroutine-from-final_suspend-r/10096047
-
-  // Prior to Visual Studio 17.9 (Feb, 2024), aka MSVC 19.39, MSVC incorrectly allocates the return
-  // buffer for await_suspend calls within the suspended coroutine frame. When the suspended
-  // coroutine is destroyed within await_suspend, the continuation coroutine handle is not only used
-  // after free, but also overwritten by the debug malloc implementation when NRVO is in play.
-
-  // This workaround delays the destruction of the suspended coroutine by wrapping the continuation
-  // in another coroutine which destroys the former and transfers execution to the original
-  // continuation.
-
-  // The wrapping coroutine is thread-local and is reused within the thread for each
-  // destroy-and-continue sequence. The wrapping coroutine itself is destroyed at thread exit.
-
-  namespace __destroy_and_continue_msvc
+  namespace __detail
   {
-    struct __task
+    struct __synthetic_coro_frame
     {
-      struct promise_type
-      {
-        __task get_return_object() noexcept
-        {
-          return {__std::coroutine_handle<promise_type>::from_promise(*this)};
-        }
-
-        static std::suspend_never initial_suspend() noexcept
-        {
-          return {};
-        }
-
-        static std::suspend_never final_suspend() noexcept
-        {
-          STDEXEC_ASSERT(!"Should never get here");
-          return {};
-        }
-
-        static void return_void() noexcept
-        {
-          STDEXEC_ASSERT(!"Should never get here");
-        }
-
-        static void unhandled_exception() noexcept
-        {
-          STDEXEC_ASSERT(!"Should never get here");
-        }
-      };
-
-      __std::coroutine_handle<> __coro_;
-    };
+      void (*__resume_)(void*) noexcept;
+      // we never invoke __destroy_ so a no-op implementation is fine; we've chosen the
+      // address of a no-op function rather than nullptr in case some rogue awaitable
+      // *does* invoke destroy on the synthesized handle that it receives in its
+      // await_suspend function
+      void (*__destroy_)(void*) noexcept = &__noop_destroy;
 
-    struct __continue_t
-    {
-      static constexpr bool await_ready() noexcept
+      static void __noop_destroy(void*) noexcept
       {
-        return false;
+        STDEXEC_ASSERT(!"Attempt to destroy a synthetic coroutine!");
       }
+    };
 
-      __std::coroutine_handle<> await_suspend(__std::coroutine_handle<>) noexcept
-      {
-        return __continue_;
-      }
+    static constexpr std::ptrdiff_t __coro_promise_offset = static_cast<std::ptrdiff_t>(
+      sizeof(__synthetic_coro_frame));
+  }  // namespace __detail
+
+#  if STDEXEC_MSVC() && STDEXEC_MSVC_VERSION <= 1939
+  // MSVCBUG https://developercommunity.visualstudio.com/t/destroy-coroutine-from-final_suspend-r/10096047
 
-      static void await_resume() noexcept {}
+  // Prior to Visual Studio 17.9 (Feb, 2024), aka MSVC 19.39, MSVC incorrectly allocates
+  // the return buffer for await_suspend calls within the suspended coroutine frame. When
+  // the suspended coroutine is destroyed within await_suspend, the continuation coroutine
+  // handle is not only used after free, but also overwritten by the debug malloc
+  // implementation when NRVO is in play.
 
-      __std::coroutine_handle<> __continue_;
-    };
+  // This workaround delays the destruction of the suspended coroutine by wrapping the
+  // continuation in another "synthetic" coroutine the resumes the continuation and *then*
+  // destroys the suspended coroutine.
 
-    struct __context
-    {
-      __std::coroutine_handle<> __destroy_;
-      __std::coroutine_handle<> __continue_;
-    };
+  // The wrapping coroutine frame is thread-local and reused within the thread for each
+  // destroy-and-continue sequence.
+
+  struct __destroy_and_continue_frame : __detail::__synthetic_coro_frame
+  {
+    constexpr __destroy_and_continue_frame() noexcept
+      : __detail::__synthetic_coro_frame{&__destroy_and_continue_frame::__resume}
+    {}
 
-    inline __task __co_impl(__context& __c)
+    static void __resume(void* __address) noexcept
     {
-      while (true)
-      {
-        co_await __continue_t{__c.__continue_};
-        __c.__destroy_.destroy();
-      }
+      // Make a local copy of the promise to ensure we can safely destroy the suspended
+      // coroutine after resuming the continuation.
+      auto __promise = static_cast<__destroy_and_continue_frame*>(__address)->__promise_;
+      STDEXEC::__coroutine_resume_nothrow(__promise.__continue_);
+      STDEXEC::__coroutine_destroy_nothrow(__promise.__destroy_);
     }
 
-    struct __context_and_coro
+    struct __promise
     {
-      __context_and_coro()
-      {
-        __context_.__continue_ = __std::noop_coroutine();
-        __coro_                = __co_impl(__context_).__coro_;
-      }
-
-      ~__context_and_coro()
-      {
-        __coro_.destroy();
-      }
-
-      __context                 __context_;
-      __std::coroutine_handle<> __coro_;
-    };
+      __std::coroutine_handle<> __destroy_{};
+      __std::coroutine_handle<> __continue_{};
+    } __promise_;
+  };
 
-    inline __std::coroutine_handle<>
-    __impl(__std::coroutine_handle<> __destroy, __std::coroutine_handle<> __continue)
-    {
-      static thread_local __context_and_coro __c;
-      __c.__context_.__destroy_  = __destroy;
-      __c.__context_.__continue_ = __continue;
-      return __c.__coro_;
-    }
-  }  // namespace __destroy_and_continue_msvc
+  inline auto __coroutine_destroy_and_continue(__std::coroutine_handle<> __destroy,            //
+                                               __std::coroutine_handle<> __continue) noexcept  //
+    -> __std::coroutine_handle<>
+  {
+    static constinit thread_local __destroy_and_continue_frame __fr;
+    __fr.__promise_.__destroy_  = __destroy;
+    __fr.__promise_.__continue_ = __continue;
+    return __std::coroutine_handle<>::from_address(&__fr);
+  }
 
 #    define STDEXEC_CORO_DESTROY_AND_CONTINUE(__destroy, __continue)                      \
-       (::STDEXEC::__destroy_and_continue_msvc::__impl(__destroy, __continue))
+       ::STDEXEC::__coroutine_destroy_and_continue(__destroy, __continue)
 #  else
 #    define STDEXEC_CORO_DESTROY_AND_CONTINUE(__destroy, __continue)                      \
        (__destroy.destroy(), __continue)