Windows abrupt shutdown & Coverage

include/boost/corosio/detail/timer.hpp

@@ -21,18 +21,15 @@
 #include <boost/capy/ex/executor_ref.hpp>
 #include <boost/capy/ex/execution_context.hpp>
 #include <boost/capy/ex/io_env.hpp>
-#include <boost/capy/concept/executor.hpp>
 
 #include <atomic>
 #include <chrono>
-#include <concepts>
 #include <coroutine>
 #include <cstddef>
 #include <limits>
 #include <new>
 #include <stop_token>
 #include <system_error>
-#include <type_traits>
 
 namespace boost::corosio::detail {
 
@@ -204,105 +201,24 @@ class BOOST_COROSIO_DECL timer : public io_object
     */
     explicit timer(capy::execution_context& ctx);
 
-    /** Construct a timer with an initial absolute expiry time.
-
-        @param ctx The execution context that will own this timer. It
-            must be a corosio io_context; otherwise the constructor
-            throws (a timer service is required).
-        @param t The initial expiry time point.
-
-        @throws std::logic_error if @p ctx is not an io_context.
-    */
-    timer(capy::execution_context& ctx, time_point t);
-
-    /** Construct a timer with an initial relative expiry time.
-
-        @param ctx The execution context that will own this timer. It
-            must be a corosio io_context; otherwise the constructor
-            throws (a timer service is required).
-        @param d The initial expiry duration relative to now.
-
-        @throws std::logic_error if @p ctx is not an io_context.
-    */
-    template<class Rep, class Period>
-    timer(capy::execution_context& ctx, std::chrono::duration<Rep, Period> d)
-        : timer(ctx)
-    {
-        expires_after(d);
-    }
-
-    /** Construct a timer from an executor.
-
-        The timer is associated with the executor's context, which must
-        be a corosio io_context.
-
-        @param ex The executor whose context will own this timer.
-
-        @throws std::logic_error if the executor's context is not an
-            io_context.
-    */
-    template<class Ex>
-        requires(!std::same_as<std::remove_cvref_t<Ex>, timer>) &&
-        capy::Executor<Ex>
-    explicit timer(Ex const& ex) : timer(ex.context())
-    {
-    }
-
-    /** Construct a timer from an executor with an absolute expiry time.
-
-        @param ex The executor whose context will own this timer.
-        @param t The initial expiry time point.
-
-        @throws std::logic_error if the executor's context is not an
-            io_context.
-    */
-    template<class Ex>
-        requires capy::Executor<Ex>
-    timer(Ex const& ex, time_point t) : timer(ex.context(), t)
-    {
-    }
-
-    /** Construct a timer from an executor with a relative expiry time.
-
-        @param ex The executor whose context will own this timer.
-        @param d The initial expiry duration relative to now.
-
-        @throws std::logic_error if the executor's context is not an
-            io_context.
-    */
-    template<class Ex, class Rep, class Period>
-        requires capy::Executor<Ex>
-    timer(Ex const& ex, std::chrono::duration<Rep, Period> d)
-        : timer(ex.context(), d)
-    {
-    }
-
     /** Move constructor.
 
-        Transfers ownership of the timer resources.
-
-        @param other The timer to move from.
+        Transfers ownership of the timer resources. Required so a
+        disengaged `std::optional<timer>` is movable; a timer is never
+        moved while a wait is published.
 
         @pre No awaitables returned by @p other's methods exist.
-        @pre The execution context associated with @p other must
-            outlive this timer.
     */
-    timer(timer&& other) noexcept;
+    timer(timer&&) noexcept = default;
 
     /** Move assignment operator.
 
         Closes any existing timer and transfers ownership.
 
-        @param other The timer to move from.
-
         @pre No awaitables returned by either `*this` or @p other's
             methods exist.
-        @pre The execution context associated with @p other must
-            outlive this timer.
-
-        @return Reference to this timer.
     */
-    timer& operator=(timer&& other) noexcept;
+    timer& operator=(timer&&) noexcept = default;
 
     timer(timer const&)            = delete;
     timer& operator=(timer const&) = delete;

include/boost/corosio/native/detail/iocp/win_scheduler.hpp

@@ -119,6 +119,14 @@ class BOOST_COROSIO_DECL win_scheduler final
     timer_service* timer_svc_ = nullptr;
     void* iocp_;
     mutable long outstanding_work_;
+
+    // Packets in flight to the completion port that reference
+    // overlapped-op memory: kernel completions owed after a pending
+    // submission, plus successful stored-result posts. Shutdown reaps
+    // until this is zero before the services free op storage. The
+    // run-loop counter cannot serve that role: frames abandoned at
+    // teardown never return their work-guard credits.
+    mutable long pending_io_ = 0;
     mutable long stopped_;
     long stop_event_posted_;
     mutable long dispatch_required_;
@@ -334,13 +342,28 @@ win_scheduler::on_pending(overlapped_op* op) const
     // and stored the results — re-post so do_one() can dispatch. The acquire
     // on failure makes those payload writes visible, so the re-posted op
     // carries valid dwError / bytes_transferred.
+    //
+    // pending_io_ counts the packet that will dispatch this op: on CAS
+    // success the kernel's own completion will find ready_ == 1 and
+    // dispatch; on CAS failure the kernel's packet was consumed as a
+    // skip (uncounted) and the re-post is the dispatching packet. A
+    // failed re-post falls back to the deferred queue, which holds the
+    // op memory itself — no packet, no count.
     long expected = 0;
-    if (!op->ready_.compare_exchange_strong(
+    if (op->ready_.compare_exchange_strong(
             expected, 1,
             std::memory_order_acq_rel, std::memory_order_acquire))
     {
-        if (!::PostQueuedCompletionStatus(
+        ::InterlockedIncrement(&pending_io_);
+    }
+    else
+    {
+        if (::PostQueuedCompletionStatus(
                 iocp_, 0, key_result_stored, static_cast<LPOVERLAPPED>(op)))
+        {
+            ::InterlockedIncrement(&pending_io_);
+        }
+        else
         {
             std::lock_guard<win_mutex> lock(dispatch_mutex_);
             completed_ops_.push(op);
@@ -359,8 +382,12 @@ win_scheduler::on_completion(overlapped_op* op, DWORD error, DWORD bytes) const
     op->bytes_transferred = bytes;
     op->ready_.store(1, std::memory_order_release);
 
-    if (!::PostQueuedCompletionStatus(
+    if (::PostQueuedCompletionStatus(
             iocp_, 0, key_result_stored, static_cast<LPOVERLAPPED>(op)))
+    {
+        ::InterlockedIncrement(&pending_io_);
+    }
+    else
     {
         std::lock_guard<win_mutex> lock(dispatch_mutex_);
         completed_ops_.push(op);
@@ -573,6 +600,7 @@ win_scheduler::do_one(unsigned long timeout_ms)
                         std::memory_order_acq_rel,
                         std::memory_order_acquire))
                 {
+                    ::InterlockedDecrement(&pending_io_);
                     ov_op->complete(
                         this, ov_op->bytes_transferred, ov_op->dwError);
                     work_finished();
@@ -725,56 +753,89 @@ win_scheduler::shutdown()
         timer_svc_->shutdown();
 
     // Same problem for the auxiliary wait reactor: ops parked in it
-    // hold work_started credit. Stop the reactor early so its loop
-    // drains them as cancelled and the work counter can reach zero.
+    // owe completion packets. Stop the reactor early so its loop
+    // posts them as cancelled and the pending count can reach zero.
     if (wait_reactor_ready_.load(std::memory_order_acquire))
         wait_reactor_->stop();
 
-    while (::InterlockedExchangeAdd(&outstanding_work_, 0) > 0)
+    // Reap every packet still owed to the port before the services
+    // free the op memory those packets reference. Work-guard credits,
+    // posted handlers, and queued continuations have no bearing here.
+    while (::InterlockedExchangeAdd(&pending_io_, 0) > 0)
     {
         op_queue ops;
         {
             std::lock_guard<win_mutex> lock(dispatch_mutex_);
             ops.splice(completed_ops_);
         }
 
-        if (!ops.empty())
+        // Deferred-queue entries are process-owned (failed-post
+        // fallbacks and posted handlers); no packet references them.
+        while (auto* h = ops.pop())
+            h->destroy();
+
+        DWORD bytes;
+        ULONG_PTR key;
+        LPOVERLAPPED overlapped;
+        ::GetQueuedCompletionStatus(
+            iocp_, &bytes, &key, &overlapped,
+            iocp::shutdown_drain_timeout_ms);
+        if (overlapped)
         {
-            while (auto* h = ops.pop())
+            if (key == key_posted)
+            {
+                auto* op = reinterpret_cast<scheduler_op*>(overlapped);
+                op->destroy();
+            }
+            else if (key == key_continuation)
+            {
+                // Drain without resuming: destroy the parked frame.
+                auto* c = reinterpret_cast<capy::continuation*>(overlapped);
+                if (c->h)
+                    c->h.destroy();
+            }
+            else
             {
-                ::InterlockedDecrement(&outstanding_work_);
-                h->destroy();
+                ::InterlockedDecrement(&pending_io_);
+                auto* op = overlapped_to_op(overlapped);
+                op->destroy();
             }
         }
+    }
+
+    // Final sweep: packets can sit in the port or deferred queue
+    // without a pending_io_ count (posted directly against the
+    // handle). Destroy them so service teardown does not free state
+    // they still reference.
+    for (;;)
+    {
+        op_queue ops;
+        {
+            std::lock_guard<win_mutex> lock(dispatch_mutex_);
+            ops.splice(completed_ops_);
+        }
+        while (auto* h = ops.pop())
+            h->destroy();
+
+        DWORD bytes;
+        ULONG_PTR key;
+        LPOVERLAPPED overlapped;
+        ::GetQueuedCompletionStatus(iocp_, &bytes, &key, &overlapped, 0);
+        if (!overlapped)
+            break;
+        if (key == key_posted)
+        {
+            reinterpret_cast<scheduler_op*>(overlapped)->destroy();
+        }
+        else if (key == key_continuation)
+        {
+            auto* c = reinterpret_cast<capy::continuation*>(overlapped);
+            if (c->h)
+                c->h.destroy();
+        }
         else
         {
-            DWORD bytes;
-            ULONG_PTR key;
-            LPOVERLAPPED overlapped;
-            ::GetQueuedCompletionStatus(
-                iocp_, &bytes, &key, &overlapped,
-                iocp::shutdown_drain_timeout_ms);
-            if (overlapped)
-            {
-                ::InterlockedDecrement(&outstanding_work_);
-                if (key == key_posted)
-                {
-                    auto* op = reinterpret_cast<scheduler_op*>(overlapped);
-                    op->destroy();
-                }
-                else if (key == key_continuation)
-                {
-                    // Drain without resuming: destroy the parked frame.
-                    auto* c = reinterpret_cast<capy::continuation*>(overlapped);
-                    if (c->h)
-                        c->h.destroy();
-                }
-                else
-                {
-                    auto* op = overlapped_to_op(overlapped);
-                    op->destroy();
-                }
-            }
+            overlapped_to_op(overlapped)->destroy();
         }
     }
 }

include/boost/corosio/native/detail/iocp/win_timers.hpp

@@ -53,23 +53,20 @@ make_win_timers(void* iocp_handle, long* dispatch_required);
 } // namespace boost::corosio::detail
 
 // Include concrete implementations needed by make_win_timers
-#include <boost/corosio/native/detail/iocp/win_timers_nt.hpp>
 #include <boost/corosio/native/detail/iocp/win_timers_thread.hpp>
 
 namespace boost::corosio::detail {
 
 inline std::unique_ptr<win_timers>
 make_win_timers(void* iocp_handle, long* dispatch_required)
 {
-    // Thread-based is faster; NT API requires one-shot re-association per
-    // wakeup which tanks performance. See timers_nt.hpp for details.
+    // Thread-based over NtAssociateWaitCompletionPacket: the NT wait
+    // packet is one-shot, so it must be re-associated (SetWaitableTimer
+    // + NtAssociateWaitCompletionPacket) after every scheduler wakeup
+    // even in timer-free workloads, costing ~60% CPU overhead. Skipping
+    // the re-association is not a fix: a spent packet never fires again
+    // and pending timers hang the scheduler.
     return std::make_unique<win_timers_thread>(iocp_handle, dispatch_required);
-
-#if 0
-    // NT native API (Windows 8+)
-    if (auto p = win_timers_nt::try_create(iocp_handle, dispatch_required))
-        return p;
-#endif
 }
 
 } // namespace boost::corosio::detail