{"id":111016,"date":"2025-03-28T07:00:00","date_gmt":"2025-03-28T14:00:00","guid":{"rendered":"https:\/\/devblogs.microsoft.com\/oldnewthing\/?p=111016"},"modified":"2025-03-28T06:45:16","modified_gmt":"2025-03-28T13:45:16","slug":"20250328-00","status":"publish","type":"post","link":"https:\/\/devblogs.microsoft.com\/oldnewthing\/20250328-00\/?p=111016","title":{"rendered":"Fixing exception safety in our task_sequencer<\/CODE>"},"content":{"rendered":"

Some time ago, we developed a task_sequencer<\/code> class<\/a> for running asynchronous operations in sequence. There’s a problem with the implementation of Queue\u00adTask\u00adAsync<\/code>: What happens if an exception occurs?<\/p>\n

Let’s look at the various places an exception can occur in Queue\u00adTask\u00adAsync<\/code>.<\/p>\n

    template<typename Maker>\r\n    auto QueueTaskAsync(Maker&& maker) ->decltype(maker())\r\n    {\r\n        auto current = std::make_shared<chained_task>();<\/span>\r\n        auto previous = [&]\r\n        {\r\n            winrt::slim_lock_guard guard(m_mutex);\r\n            return std::exchange(m_latest, current); \u2190 oops<\/span>\r\n        }();\r\n\r\n        suspender suspend;\r\n\r\n        using Async = decltype(maker());\r\n        auto task = [](auto&& current, auto&& makerParam,\r\n                       auto&& contextParam, auto& suspend)\r\n                    -> Async\r\n        {\r\n            completer completer{ std::move(current) };\r\n            auto maker = std::move(makerParam);\r\n            auto context = std::move(contextParam);\r\n\r\n            co_await suspend;\r\n            co_await context;\r\n            co_return co_await maker();\r\n        }(current, std::forward<Maker>(maker),<\/span>\r\n          winrt::apartment_context(), suspend);\r\n\r\n        previous->continue_with(suspend.handle);\r\n\r\n        return task;\r\n    }\r\n<\/pre>\n
If an exception occurs at make_shared<\/code>, then no harm is done because we haven’t done anything yet.<\/p>\n
If an exception occurs when starting the lambda task, then we are in trouble. We have already linked the current<\/code> onto m_latest<\/code>, but we will never call continue_with()<\/code>, so the chain of tasks stops making progress.<\/p>\n
To fix this, we need to delay hooking up the current<\/code> to the chain of chained_task<\/code>s until we are sure that we have a task to chain. This means that the std::exchange(m_latest, current);<\/code> needs to wait until the task is created. But we can’t just swap the order of the two operations because the task does a std::move(current)<\/code>, which empties out the current<\/code><\/p>\n
    template<typename Maker>\r\n    auto QueueTaskAsync(Maker&& maker) ->decltype(maker())\r\n    {\r\n        auto current = std::make_shared<chained_task>();\r\n\r\n        suspender suspend;\r\n\r\n        using Async = decltype(maker());\r\n        auto task = [](auto&& current, auto&& makerParam,\r\n                       auto&& contextParam, auto& suspend)\r\n                    -> Async\r\n        {\r\n            completer completer{ std::move(current) };\r\n            auto maker = std::move(makerParam);\r\n            auto context = std::move(contextParam);\r\n\r\n            co_await suspend;\r\n            co_await context;\r\n            co_return co_await maker();\r\n        }(current, std::forward<Maker>(maker),\r\n          winrt::apartment_context(), suspend);\r\n\r\n        \/\/ moved this to after we create the task                <\/span>\r\n        auto previous = [&]                                      <\/span>\r\n        {                                                        <\/span>\r\n            winrt::slim_lock_guard guard(m_mutex);               <\/span>\r\n            return std::exchange(m_latest, current); \u2190 still oops<\/span>\r\n        }();                                                     <\/span>\r\n\r\n        previous->continue_with(suspend.handle);\r\n\r\n        return task;\r\n    }\r\n<\/pre>\n
The last person to use the current<\/code> is allowed to move from it, so we have to move the move.<\/p>\n
    template<typename Maker>\r\n    auto QueueTaskAsync(Maker&& maker) ->decltype(maker())\r\n    {\r\n        auto current = std::make_shared<chained_task>();\r\n\r\n        suspender suspend;\r\n\r\n        using Async = decltype(maker());\r\n        auto task = [](auto&& current, auto&& makerParam,\r\n                       auto&& contextParam, auto& suspend)\r\n                    -> Async\r\n        {\r\n            completer completer{ current<\/span> };\r\n            auto maker = std::move(makerParam);\r\n            auto context = std::move(contextParam);\r\n\r\n            co_await suspend;\r\n            co_await context;\r\n            co_return co_await maker();\r\n        }(current, std::forward<Maker>(maker),\r\n          winrt::apartment_context(), suspend);\r\n\r\n        auto previous = [&]\r\n        {\r\n            winrt::slim_lock_guard guard(m_mutex);\r\n            return std::exchange(m_latest, std::move<\/span>(current));\r\n        }();\r\n\r\n        previous->continue_with(suspend.handle);\r\n\r\n        return task;\r\n    }\r\n<\/pre>\n
While we’re here, we may as well do some cleaning up. For example, there was no need to create the apartment_context<\/code> outside the lambda and move it into the lambda locals. We can just create it as a lambda local.<\/p>\n
        auto task = [](auto&& current, auto&& makerParam,\r\n                       \/* auto&& contextParam,<\/span> *\/<\/span> auto& suspend)\r\n                    -> Async\r\n        {\r\n            completer completer{ current };\r\n            auto maker = std::move(makerParam);\r\n            auto context = winrt::apartment_context()<\/span>;\r\n\r\n            co_await suspend;\r\n            co_await context;\r\n            co_return co_await maker();\r\n        }(current, std::forward<Maker>(maker),\r\n          \/* winrt::apartment_context(),<\/span> *\/<\/span> suspend);\r\n<\/pre>\n
And instead of passing the other lambda parameters by reference, we can use a reference capture. We just have to be careful to copy them to locals before our first co_await<\/code>. (And there was another bug in the original version: It moved the maker<\/code> into the local rather than forwarding it.)<\/p>\n
        auto task = [&<\/span>]() -> Async\r\n        {\r\n            completer completer{ current };\r\n            auto local_maker<\/span> = std::forward<Maker>(maker)<\/span>;\r\n            auto context = winrt::apartment_context();\r\n\r\n            co_await suspend;\r\n            co_await context;\r\n            co_return co_await local_maker<\/span>();\r\n        }(\/* \u27e6 ... \u27e7<\/span> *\/<\/span>);\r\n<\/pre>\n
And now that the last consumer of current<\/code> is the Queue\u00adTask\u00adAsync<\/code> function itself, we can swap it instead of moving it out of one variable and into another.<\/p>\n
        \/* auto previous = [&]<\/span> *\/<\/span>\r\n        {\r\n            winrt::slim_lock_guard guard(m_mutex);\r\n            m_latest.swap(current);<\/span>\r\n        }\/* ()<\/span> *\/<\/span>\r\n\r\n        current<\/span>->continue_with(suspend.handle);\r\n<\/pre>\n
The name current<\/code> is confusing for a variable that actually holds the previous<\/code> entry, so let’s give it a generic name node<\/code>.<\/p>\n
The result of all this cleanup is the following class. Everything outside Queue\u00adTask\u00adAsync<\/code> is unchanged, but I include it here for copy-paste-friendliness.<\/p>\n
struct task_sequencer\r\n{\r\n    task_sequencer() = default;\r\n    task_sequencer(const task_sequencer&) = delete;\r\n    void operator=(const task_sequencer&) = delete;\r\n\r\nprivate:\r\n    using coro_handle = std::experimental::coroutine_handle<>;\r\n\r\n    struct suspender\r\n    {\r\n        bool await_ready() const noexcept { return false; }\r\n        void await_suspend(coro_handle h)\r\n            noexcept { handle = h; }\r\n        void await_resume() const noexcept { }\r\n\r\n        coro_handle handle;\r\n    };\r\n\r\n    static void* completed()\r\n    { return reinterpret_cast<void*>(1); }\r\n\r\n    struct chained_task\r\n    {\r\n        chained_task(void* state = nullptr) : next(state) {}\r\n\r\n        void continue_with(coro_handle h) {\r\n            if (next.exchange(h.address(),\r\n                        std::memory_order_acquire) != nullptr) {\r\n                h();\r\n            }\r\n        }\r\n\r\n        void complete() {\r\n            auto resume = next.exchange(completed());\r\n            if (resume) {\r\n                coro_handle::from_address(resume).resume();\r\n            }\r\n        }\r\n\r\n        std::atomic<void*> next;\r\n    };\r\n\r\n    struct completer\r\n    {\r\n        ~completer()\r\n        {\r\n            chain->complete();\r\n        }\r\n        std::shared_ptr<chained_task> chain;\r\n    };\r\n\r\n    winrt::slim_mutex m_mutex;\r\n    std::shared_ptr<chained_task> m_latest =\r\n        std::make_shared<chained_task>(completed());\r\n\r\npublic:\r\n    template<typename Maker>\r\n    auto QueueTaskAsync(Maker&& maker) ->decltype(maker())\r\n    {\r\n        auto node = std::make_shared<chained_task>();\r\n\r\n        suspender suspend;\r\n\r\n        using Async = decltype(maker());\r\n        auto task = [&]() -> Async\r\n        {\r\n            completer completer{ current };\r\n            auto local_maker = std::forward<Maker>(maker);\r\n            auto context = winrt::apartment_context();\r\n\r\n            co_await suspend;\r\n            co_await context;\r\n            co_return co_await local_maker();\r\n        }();\r\n\r\n        {\r\n            winrt::slim_lock_guard guard(m_mutex);\r\n            m_latest.swap(node);\r\n        }\r\n\r\n        node->continue_with(suspend.handle);\r\n\r\n        return task;\r\n    }\r\n};\r\n<\/pre>\n","protected":false},"excerpt":{"rendered":"
Exception safety, the forgotten requirement.<\/p>\n","protected":false},"author":1069,"featured_media":111744,"comment_status":"open","ping_status":"closed","sticky":false,"template":"","format":"standard","meta":{"_acf_changed":false,"footnotes":""},"categories":[1],"tags":[25],"class_list":["post-111016","post","type-post","status-publish","format-standard","has-post-thumbnail","hentry","category-oldnewthing","tag-code"],"acf":[],"blog_post_summary":"
Exception safety, the forgotten requirement.<\/p>\n","_links":{"self":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts\/111016","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/users\/1069"}],"replies":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/comments?post=111016"}],"version-history":[{"count":0,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/posts\/111016\/revisions"}],"wp:featuredmedia":[{"embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/media\/111744"}],"wp:attachment":[{"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/media?parent=111016"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/categories?post=111016"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/devblogs.microsoft.com\/oldnewthing\/wp-json\/wp\/v2\/tags?post=111016"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}