Awaiting an already-completed IAsyncXxx does not consume stack in C++20 mode (#1040)

Author: oldnewthing

cppwinrt.props

@@ -55,12 +55,14 @@
       <WarningLevel>Level4</WarningLevel>
       <SDLCheck>true</SDLCheck>
       <ConformanceMode>true</ConformanceMode>
-      <LanguageStandard>stdcpp17</LanguageStandard>
+      <LanguageStandard Condition="'$(CppWinRTLanguageStandard)'==''">stdcpp17</LanguageStandard>
+      <LanguageStandard Condition="'$(CppWinRTLanguageStandard)'=='latest'">stdcpplatest</LanguageStandard>
       <PrecompiledHeader>Use</PrecompiledHeader>
       <PrecompiledHeaderFile>pch.h</PrecompiledHeaderFile>
       <PreprocessorDefinitions>CPPWINRT_VERSION_STRING="$(CppWinRTBuildVersion)";%(PreprocessorDefinitions)</PreprocessorDefinitions>
       <MultiProcessorCompilation>true</MultiProcessorCompilation>
-      <AdditionalOptions>/await /bigobj</AdditionalOptions>
+      <AdditionalOptions>/bigobj</AdditionalOptions>
+      <AdditionalOptions Condition="'$(CppWinRTLanguageStandard)'==''">/await %(AdditionalOptions)</AdditionalOptions>
       <AdditionalOptions Condition="'$(Clang)'=='1'">-Wno-unused-command-line-argument -fno-delayed-template-parsing -Xclang -fcoroutines-ts -mcx16</AdditionalOptions>
     </ClCompile>
     <Link>

strings/base_coroutine_foundation.h

@@ -98,34 +98,45 @@ namespace winrt::impl
         return async.GetResults();
     }
 
+    template<typename Awaiter>
     struct disconnect_aware_handler
     {
-        disconnect_aware_handler(coroutine_handle<> handle, int32_t* failure) noexcept
-            : m_handle(handle), m_failure(failure) { }
+        disconnect_aware_handler(Awaiter* awaiter, coroutine_handle<> handle) noexcept
+            : m_awaiter(awaiter), m_handle(handle) { }
 
         disconnect_aware_handler(disconnect_aware_handler&& other) noexcept
             : m_context(std::move(other.m_context))
-            , m_handle(std::exchange(other.m_handle, {}))
-            , m_failure(other.m_failure) { }
+            , m_awaiter(std::exchange(other.m_awaiter, {}))
+            , m_handle(std::exchange(other.m_handle, {})) { }
 
         ~disconnect_aware_handler()
         {
             if (m_handle) Complete();
         }
 
-        void operator()()
+        template<typename Async>
+        void operator()(Async&&, Windows::Foundation::AsyncStatus status)
         {
+            m_awaiter->status = status;
             Complete();
         }
 
     private:
         resume_apartment_context m_context;
+        Awaiter* m_awaiter;
         coroutine_handle<> m_handle;
-        int32_t* m_failure;
 
         void Complete()
         {
-            resume_apartment(m_context, std::exchange(m_handle, {}), m_failure);
+            if (m_awaiter->suspending.exchange(false, std::memory_order_release))
+            {
+                m_handle = nullptr; // resumption deferred to await_suspend
+            }
+            else
+            {
+                resume_apartment(m_context, std::exchange(m_handle, {}), &m_awaiter->failure);
+            }
+
         }
     };
 
@@ -138,6 +149,7 @@ namespace winrt::impl
         Async const& async;
         Windows::Foundation::AsyncStatus status = Windows::Foundation::AsyncStatus::Started;
         int32_t failure = 0;
+        std::atomic<bool> suspending{ true };
 
         void enable_cancellation(cancellable_promise* promise)
         {
@@ -152,14 +164,18 @@ namespace winrt::impl
             return false;
         }
 
-        void await_suspend(coroutine_handle<> handle)
+        auto await_suspend(coroutine_handle<> handle)
         {
             auto extend_lifetime = async;
-            async.Completed([this, handler = disconnect_aware_handler{ handle, &failure }](auto&&, auto operation_status) mutable
+            async.Completed(disconnect_aware_handler(this, handle));
+#ifdef _RESUMABLE_FUNCTIONS_SUPPORTED
+            if (!suspending.exchange(false, std::memory_order_acquire))
             {
-                status = operation_status;
-                handler();
-            });
+                handle.resume();
+            }
+#else
+            return suspending.exchange(false, std::memory_order_acquire);
+#endif
         }
 
         auto await_resume() const

test/test/await_completed.cpp

@@ -0,0 +1,69 @@
+#include "pch.h"
+
+using namespace winrt;
+using namespace Windows::Foundation;
+
+namespace
+{
+    //
+    // Checks that awaiting an already-completed async operation
+    // does not consume additional stack.
+    //
+    IAsyncAction AlreadyCompleted()
+    {
+        co_return;
+    }
+
+#ifdef _MSC_VER
+    __declspec(noinline) uintptr_t approximate_stack_pointer()
+    {
+        return reinterpret_cast<uintptr_t>(_AddressOfReturnAddress());
+    }
+#else
+    // gcc, clang, and icc all support this.
+    __attribute__((noinline)) uintptr_t approximate_stack_pointer()
+    {
+        return reinterpret_cast<uintptr_t>(__builtin_frame_address(0));
+    }
+#endif
+
+    // Simple awaiter that (inefficiently) resumes from inside
+    // await_suspend, for the purpose of measuring how much stack it consumes.
+    // This is the best we can do with MSVC prerelease coroutines prior to 16.11.
+    struct resume_sync_from_await_suspend
+    {
+        bool await_ready() { return false; }
+        void await_suspend(winrt::impl::coroutine_handle<> h) { h(); }
+        void await_resume() { }
+    };
+
+    IAsyncAction SyncCompletion()
+    {
+        uintptr_t initial = approximate_stack_pointer();
+        co_await resume_sync_from_await_suspend();
+        uintptr_t sync_usage = initial - approximate_stack_pointer();
+
+        initial = approximate_stack_pointer();
+        co_await AlreadyCompleted();
+        uintptr_t consumed = initial - approximate_stack_pointer();
+#ifdef _RESUMABLE_FUNCTIONS_SUPPORTED
+        // This branch is taken only for MSVC prerelease coroutines.
+        //
+        // MSVC prerelease coroutines prior to 16.11 do not implement "bool await_suspend" reliably,
+        // so we can't use it impl::await_adapter. We must resume inline inside await_suspend,
+        // so there is a small amount of stack usage. (Pre-16.11 and post-16.11 prerelease coroutines
+        // are interoperable, so we cannot change behavior based on which compiler we are using,
+        // because that would introduce ODR violations. Our first opportunity to change behavior
+        // is the ABI breaking change with MSVC standard-conforming coroutines.)
+        REQUIRE(consumed <= sync_usage);
+#else
+        // MSVC standard-conforming coroutines (as well as gcc and clang coroutines)
+        // support "bool await_suspend" just fine.
+        REQUIRE(consumed == 0);
+#endif
+    }
+}
+TEST_CASE("await_completed_await")
+{
+    SyncCompletion().get();
+}

test/test/test.vcxproj

@@ -296,6 +296,7 @@
     <ClCompile Include="async_completed.cpp" />
     <ClCompile Include="async_propagate_cancel.cpp" />
     <ClCompile Include="async_ref_result.cpp" />
+    <ClCompile Include="await_completed.cpp" />
     <ClCompile Include="box_array.cpp" />
     <ClCompile Include="box_delegate.cpp" />
     <ClCompile Include="box_guid.cpp" />

test/test_cpp20/await_completed.cpp

@@ -0,0 +1,45 @@
+#include "pch.h"
+
+using namespace winrt;
+using namespace Windows::Foundation;
+
+namespace
+{
+    //
+    // Checks that awaiting an already-completed async operation
+    // does not consume additional stack.
+    //
+    IAsyncAction AlreadyCompleted()
+    {
+        co_return;
+    }
+
+#ifdef _MSC_VER
+    __declspec(noinline) uintptr_t approximate_stack_pointer()
+    {
+        return reinterpret_cast<uintptr_t>(_AddressOfReturnAddress());
+    }
+#else
+    // gcc, clang, and icc all support this.
+    __attribute__((noinline)) uintptr_t approximate_stack_pointer()
+    {
+        return reinterpret_cast<uintptr_t>(__builtin_frame_address(0));
+    }
+#endif
+
+#ifdef _RESUMABLE_FUNCTIONS_SUPPORTED
+#error C++20 test should be compiled in C++20 mode.
+#endif
+
+    IAsyncAction SyncCompletion()
+    {
+        uintptr_t initial = approximate_stack_pointer();
+        co_await AlreadyCompleted();
+        uintptr_t consumed = initial - approximate_stack_pointer();
+        REQUIRE(consumed == 0);
+    }
+}
+TEST_CASE("await_completed_await")
+{
+    SyncCompletion().get();
+}

test/test_cpp20/test_cpp20.vcxproj

@@ -40,6 +40,7 @@
     <RootNamespace>unittests</RootNamespace>
     <ProjectName>test_cpp20</ProjectName>
     <WindowsTargetPlatformVersion>10.0</WindowsTargetPlatformVersion>
+    <CppWinRTLanguageStandard>latest</CppWinRTLanguageStandard>
   </PropertyGroup>
   <Import Project="$(SolutionDir)\cppwinrt.props" />
   <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'" Label="Configuration">
@@ -130,7 +131,6 @@
       <AdditionalIncludeDirectories>$(OutputPath);Generated Files;..\</AdditionalIncludeDirectories>
       <PreprocessorDefinitions>NOMINMAX;_MBCS;%(PreprocessorDefinitions)</PreprocessorDefinitions>
       <RuntimeLibrary>MultiThreaded</RuntimeLibrary>
-      <LanguageStandard>stdcpplatest</LanguageStandard>
     </ClCompile>
     <Link>
       <SubSystem>Console</SubSystem>
@@ -152,7 +152,6 @@
       <AdditionalIncludeDirectories>$(OutputPath);Generated Files;..\</AdditionalIncludeDirectories>
       <PreprocessorDefinitions>NOMINMAX;_MBCS;%(PreprocessorDefinitions)</PreprocessorDefinitions>
       <RuntimeLibrary>MultiThreadedDebug</RuntimeLibrary>
-      <LanguageStandard>stdcpplatest</LanguageStandard>
     </ClCompile>
     <Link>
       <SubSystem>Console</SubSystem>
@@ -172,7 +171,6 @@
       <AdditionalIncludeDirectories>$(OutputPath);Generated Files;..\</AdditionalIncludeDirectories>
       <PreprocessorDefinitions>NOMINMAX;_MBCS;%(PreprocessorDefinitions)</PreprocessorDefinitions>
       <RuntimeLibrary>MultiThreadedDebug</RuntimeLibrary>
-      <LanguageStandard>stdcpplatest</LanguageStandard>
     </ClCompile>
     <Link>
       <SubSystem>Console</SubSystem>
@@ -192,7 +190,6 @@
       <AdditionalIncludeDirectories>$(OutputPath);Generated Files;..\</AdditionalIncludeDirectories>
       <PreprocessorDefinitions>NOMINMAX;_MBCS;%(PreprocessorDefinitions)</PreprocessorDefinitions>
       <RuntimeLibrary>MultiThreadedDebug</RuntimeLibrary>
-      <LanguageStandard>stdcpplatest</LanguageStandard>
     </ClCompile>
     <Link>
       <SubSystem>Console</SubSystem>
@@ -212,7 +209,6 @@
       <AdditionalIncludeDirectories>$(OutputPath);Generated Files;..\</AdditionalIncludeDirectories>
       <PreprocessorDefinitions>NOMINMAX;_MBCS;%(PreprocessorDefinitions)</PreprocessorDefinitions>
       <RuntimeLibrary>MultiThreadedDebug</RuntimeLibrary>
-      <LanguageStandard>stdcpplatest</LanguageStandard>
     </ClCompile>
     <Link>
       <SubSystem>Console</SubSystem>
@@ -234,7 +230,6 @@
       <AdditionalIncludeDirectories>$(OutputPath);Generated Files;..\</AdditionalIncludeDirectories>
       <PreprocessorDefinitions>NOMINMAX;_MBCS;%(PreprocessorDefinitions)</PreprocessorDefinitions>
       <RuntimeLibrary>MultiThreaded</RuntimeLibrary>
-      <LanguageStandard>stdcpplatest</LanguageStandard>
     </ClCompile>
     <Link>
       <SubSystem>Console</SubSystem>
@@ -258,7 +253,6 @@
       <AdditionalIncludeDirectories>$(OutputPath);Generated Files;..\</AdditionalIncludeDirectories>
       <PreprocessorDefinitions>NOMINMAX;_MBCS;%(PreprocessorDefinitions)</PreprocessorDefinitions>
       <RuntimeLibrary>MultiThreaded</RuntimeLibrary>
-      <LanguageStandard>stdcpplatest</LanguageStandard>
     </ClCompile>
     <Link>
       <SubSystem>Console</SubSystem>
@@ -282,7 +276,6 @@
       <AdditionalIncludeDirectories>$(OutputPath);Generated Files;..\</AdditionalIncludeDirectories>
       <PreprocessorDefinitions>NOMINMAX;_MBCS;%(PreprocessorDefinitions)</PreprocessorDefinitions>
       <RuntimeLibrary>MultiThreaded</RuntimeLibrary>
-      <LanguageStandard>stdcpplatest</LanguageStandard>
     </ClCompile>
     <Link>
       <SubSystem>Console</SubSystem>
@@ -302,6 +295,7 @@
     <ClInclude Include="pch.h" />
   </ItemGroup>
   <ItemGroup>
+    <ClCompile Include="await_completed.cpp" />
     <ClCompile Include="format.cpp" />
     <ClCompile Include="hstring.cpp" />
     <ClCompile Include="main.cpp">