Fix an oversight in openNursery() implementation.

There is currently another oversight in recommended way of establishing
a nursery for background operations of active objects. In this code
snippet:

    class My {
        corral::Nursery* nursery_;

        corral::Task<void> doThing() {
            co_await corral::noncancellable(something);
            nursery_->start(somethingElseInBackground());
        }

      public:
        corral::Task<void> run() {
            CORRAL_WITH_NURSERY(n) {
                nursery_ = &n;
                ScopeGuard guard([&]{ nursery_ = nullptr; });
                CORRAL_SUSPEND_FOREVER();
            };
        }

        void beginThing() {
            nursery_->start(&My::doThing, this);
        }
    };

— `My::nursery_` would get zeroed out upon cancellation request, before
the nursery and all tasks therein are actually cancelled. All tasks
in the nursery will get cancelled as well, but if a task was busy doing
any noncancellable stuff, it would resume normally. Any attempt to access
a nursery thereafter would result in a segfault, since the nursery
is already zeroed out.

This breaks reasoning: it feels natural to assume that, if a task is running
in a nursery, the nursery is available for submitting more tasks in there.
Worse, even if we announced very loudly that the above assumption does not
in fact hold, that would not provide much of any alternative for users
to dispatch the piece of work they were going to submit there
(`if (nursery_) nursery_->start(...); else /*what?*/`).

This diff addresses this by adding another nursery flavor,
`detail::BackreferencedNursery`, which holds a backreference to itself,
and can zero it out when it's about to close. To prevent any potential
footguns, such a nursery can only be constructed by `openNursery()` function.

To simplify `openNursery()` implementation, `UnsafeNursery::Awaitable`
has been moved to `Nursery` class, and templatized by nursery type,
so it can handle both `UnsafeNursery` and `BackreferencedNursery`.

corral/Nursery.h

@@ -105,6 +105,7 @@ class Nursery : private detail::TaskParent<void> {
     template <class Ret> class StartAwaitableBase;
     template <class Ret, class Callable, class... Args> class StartAwaitable;
     template <class Ret> friend class TaskStarted;
+    friend Task<void> openNursery(Nursery*&, TaskStarted<>);
 
   public:
     struct Factory;
@@ -149,6 +150,7 @@ class Nursery : private detail::TaskParent<void> {
     template <class Callable> class Scope;
 
   protected:
+    template <class NurseryT> class Awaitable;
     template <class Derived> class ParentAwaitable;
 
     Nursery() = default;
@@ -184,6 +186,9 @@ class Nursery : private detail::TaskParent<void> {
         return TaskStarted<Ret>(parent);
     }
 
+    void introspect(detail::TaskTreeCollector& c,
+                    const char* title) const noexcept;
+
   protected:
     Executor* executor_ = nullptr;
     detail::IntrusiveList<detail::BasePromise> tasks_;
@@ -223,8 +228,6 @@ class Nursery : private detail::TaskParent<void> {
 /// a deep understanding of the nature of any tasks spawned directly or
 /// indirectly into this nursery.
 class UnsafeNursery final : public Nursery, private Executor {
-    class Awaitable;
-
   public:
     template <class EventLoopT>
     explicit UnsafeNursery(EventLoopT&& eventLoop)
@@ -307,13 +310,7 @@ class UnsafeNursery final : public Nursery, private Executor {
 
     // Allow the nursery itself to be an introspection root for its executor
     void await_introspect(detail::TaskTreeCollector& c) const noexcept {
-        static_assert(std::is_base_of_v<
-                      detail::IntrusiveListItem<detail::BasePromise>,
-                      detail::BasePromise>);
-        c.node("UnsafeNursery");
-        for (auto& t : tasks_) {
-            c.child(t);
-        }
+        introspect(c, "UnsafeNursery");
     }
 };
 
@@ -357,6 +354,22 @@ template <class Ret> class TaskStarted {
 };
 
 
+/// Usable for implementing live objects, if the only things needed
+/// from their `run()` methods is establishing a nursery:
+///
+///     class MyLiveObject {
+///         corral::Nursery* nursery_;
+///       public:
+///         auto run() { return corral::openNursery(nursery_); }
+///         void startStuff() { nursery_->start(doStuff()); }
+///     };
+///
+/// The nursery pointer passed as an argument will be initialized once
+/// the nursery is opened, and reset to nullptr when the nursery
+/// is closed. Does not return until cancelled.
+Task<void> openNursery(Nursery*& ptr, TaskStarted<> started = {});
+
+
 // ------------------------------------------------------------------------------------
 // Implementation
 
@@ -707,6 +720,15 @@ inline Handle Nursery::continuation(detail::BasePromise* promise) noexcept {
     return std::noop_coroutine();
 }
 
+
+inline void Nursery::introspect(detail::TaskTreeCollector& c,
+                                const char* title) const noexcept {
+    c.node(title);
+    for (auto& t : tasks_) {
+        c.child(t);
+    }
+}
+
 //
 // Logic for binding the nursery parent to the nursery
 //
@@ -733,14 +755,21 @@ template <class Derived> class Nursery::ParentAwaitable {
     }
 };
 
-class UnsafeNursery::Awaitable : public Nursery::ParentAwaitable<Awaitable> {
-    friend ParentAwaitable;
-    friend UnsafeNursery;
-    UnsafeNursery& nursery_;
+template <class NurseryT>
+class Nursery::Awaitable
+  : public Nursery::ParentAwaitable<Awaitable<NurseryT>> {
+    friend ParentAwaitable<Awaitable<NurseryT>>;
+    friend NurseryT;
 
-    explicit Awaitable(UnsafeNursery& nursery) : nursery_(nursery) {}
+    NurseryT& nursery_;
+
+    explicit Awaitable(NurseryT& nursery) : nursery_(nursery) {}
 
   public:
+    ~Awaitable()
+        requires(std::derived_from<NurseryT, Nursery>)
+    = default;
+
     bool await_ready() const noexcept { return nursery_.executor_ == nullptr; }
     bool await_suspend(Handle h) {
         CORRAL_ASSERT(!nursery_.parent_);
@@ -773,13 +802,6 @@ class Nursery::Scope : public detail::NurseryScopeBase,
       public:
         Impl() = default;
         Impl(Impl&&) noexcept = default;
-
-        void introspect(detail::TaskTreeCollector& c) const noexcept {
-            c.node("Nursery");
-            for (auto& t : tasks_) {
-                c.child(t);
-            }
-        }
     };
 
   public:
@@ -798,7 +820,7 @@ class Nursery::Scope : public detail::NurseryScopeBase,
     }
 
     void await_introspect(detail::TaskTreeCollector& c) const noexcept {
-        nursery_.introspect(c);
+        nursery_.introspect(c, "Nursery");
     }
 
   private:
@@ -825,28 +847,43 @@ struct Nursery::Factory {
 };
 
 
-/// Usable for implementing live objects, if the only things needed
-/// from their `run()` methods is establishing a nursery:
-///
-///     class MyLiveObject {
-///         corral::Nursery* nursery_;
-///       public:
-///         auto run() { return corral::openNursery(nursery_); }
-///         void startStuff() { nursery_->start(doStuff()); }
-///     };
-///
-/// The nursery pointer passed as an argument will be initialized once
-/// the nursery is opened, and reset to nullptr when the
-/// `openNursery()` task receives a cancellation request (which may be
-/// slightly before the nursery closes). Does not return until cancelled.
-inline Task<void> openNursery(Nursery*& ptr, TaskStarted<> started = {}) {
-    CORRAL_WITH_NURSERY(nursery) {
-        ptr = &nursery;
-        detail::ScopeGuard guard([&] { ptr = nullptr; });
-        started();
-        co_await SuspendForever{};
-        co_return join; // make MSVC happy
-    };
+namespace detail {
+class BackreferencedNursery final : public Nursery {
+    friend Task<void> corral::openNursery(Nursery*&, TaskStarted<>);
+    friend Nursery::Awaitable<BackreferencedNursery>;
+
+  private /*methods*/:
+    BackreferencedNursery(Executor* executor, Nursery*& backref)
+      : backref_(backref) {
+        backref_ = this;
+        executor_ = executor;
+    }
+
+    Handle continuation(detail::BasePromise* p) noexcept override {
+        if (taskCount_ == 1 && pendingTaskCount_ == 0) {
+            backref_ = nullptr;
+        }
+        return Nursery::continuation(p);
+    }
+
+    corral::Awaitable<void> auto join() { return Awaitable(*this); }
+
+    void await_introspect(detail::TaskTreeCollector& c) const noexcept {
+        introspect(c, "Nursery");
+    }
+
+  private /*fields*/:
+    Nursery*& backref_;
+};
+} // namespace detail
+
+
+inline Task<void> openNursery(Nursery*& ptr, TaskStarted<> started /*= {}*/) {
+    Executor* ex = co_await getExecutor();
+    detail::BackreferencedNursery nursery(ex, ptr);
+    nursery.start([]() -> Task<> { co_await SuspendForever{}; });
+    started();
+    co_await nursery.join();
 }
 
 } // namespace corral

doc/03_live_objects.md

@@ -146,6 +146,18 @@ task handle. This is a more efficient way to wrap an async function
 than `co_return co_await corral::openNursery(nursery_);`, but both
 work.)
 
+`openNursery()` has another advantage over its makeshift equivalent
+shown above: with `openNursery()`, the nursery pointer will be cleared
+when the nursery is actually closed (i.e., when its last task exits),
+not merely when the cancellation request is received by the `openNursery()`
+task. That means any tasks still running in the nursery during cleanup
+may continue to use the nursery pointer to start additional tasks
+to perform whatever operations are needed in order to cleanly shut down.
+Such tasks will begin execution in an already-cancelled state (so they
+probably want to make careful use of `corral::noncancellable()`
+or `corral::untilCancelledAnd()`), but in many cases that's
+preferable to not being able to start them at all.
+
 Using such a class typically involves opening a nursery, and submitting
 the `run()` task to run there in the background:
 

doc/05_reference.md

@@ -170,7 +170,8 @@ A scope in which a dynamic number of child tasks can run.
 * `Task<void> openNursery(Nursery*& ptr)`
   : Opens a new nursery, and sets `ptr` to point to it.
   The nursery remains open until the task is cancelled, and `ptr` will be
-  reset to null at that point.
+  reset to null after all tasks in the nursery complete or cancel and
+  the nursery closes.
 
 ### UnsafeNursery
 

test/basic_test.cc

@@ -1298,6 +1298,8 @@ CORRAL_TEST_CASE("nursery-task-started") {
             co_await t.sleep(1ms);
             cancelInner.trigger();
             co_await t.sleep(1ms);
+            CATCH_CHECK(inner);
+            co_await t.sleep(5ms);
             CATCH_CHECK(!inner);
 
             co_return join;
@@ -1315,6 +1317,18 @@ CORRAL_TEST_CASE("nursery-task-started") {
     }
 }
 
+CORRAL_TEST_CASE("open-nursery-cancel") {
+    Nursery* n = nullptr;
+    CORRAL_WITH_NURSERY(n2) {
+        co_await n2.start(openNursery, std::ref(n));
+        n->start([&]() -> Task<> {
+            co_await t.sleep(1ms, noncancellable);
+            n->start([&]() -> Task<> { co_return; });
+        });
+        co_return cancel;
+    };
+}
+
 CORRAL_TEST_CASE("shared") {
     auto shared = Shared([&]() -> Task<int> {
         co_await t.sleep(5ms);