BoehmGCStackAllocator: ignore stack protection page

src/libexpr/eval.cc

@@ -233,15 +233,52 @@ class BoehmGCStackAllocator : public StackAllocator {
     std::max(boost::context::stack_traits::default_size(), static_cast<std::size_t>(8 * 1024 * 1024))
     };
 
+    // This is specific to boost::coroutines2::protected_fixedsize_stack.
+    // The stack protection page is included in sctx.size, so we have to
+    // subtract one page size from the stack size.
+    std::size_t pfss_usable_stack_size(boost::context::stack_context &sctx) {
+      return sctx.size - boost::context::stack_traits::page_size();
+    }
+
   public:
     boost::context::stack_context allocate() override {
         auto sctx = stack.allocate();
-        GC_add_roots(static_cast<char *>(sctx.sp) - sctx.size, sctx.sp);
+
+        // Ideally we'd like the collector to scan this stack, but this doesn't seem to
+        // be possible (yet?).
+        //   - Segfault while GC is pushing all stacks for marking.
+        //     This suggests that Boehm GC can't process the normal thread stack
+        //     after moving control to the coroutine.
+        //       - https://github.com/NixOS/nix/issues/3931
+        //         This issue predates evaluation of source filter function 
+        //         invocations in coroutines.
+        //       - https://github.com/NixOS/nix/issues/4178#issuecomment-728705868
+        //         Whereas this one doesn't.
+        //   - This comment is somewhat concerning.
+        //      "[Traversing stacks] must be done last, since they can
+        //       legitimately overflow the mark stack.  This is usually
+        //       done by saving the current context on the stack, and then
+        //       just tracing from the stack."
+        //     See https://github.com/ivmai/bdwgc/blob/802e47d5e8dcbddd2e80cd4587eacc5f4ffbc745/mark_rts.c#L923-L927
+        //
+        // Original implementation:
+        // // Despite what the docs warn, the only *implemented* stack direction
+        // // is "down". This is why we subtract the stack size.
+        // GC_add_roots(static_cast<char *>(sctx.sp) - pfss_usable_stack_size(sctx), sctx.sp);
+
+        // GC_disable/enable maintain a counter to support nesting.
+        GC_disable();
+
         return sctx;
     }
 
     void deallocate(boost::context::stack_context sctx) override {
-        GC_remove_roots(static_cast<char *>(sctx.sp) - sctx.size, sctx.sp);
+        // // See above
+        // GC_remove_roots(static_cast<char *>(sctx.sp) - pfss_usable_stack_size(sctx), sctx.sp);
+
+        // GC_disable/enable maintain a counter to support nesting.
+        GC_enable();
+
         stack.deallocate(sctx);
     }
 

src/libexpr/primops.cc

@@ -772,6 +772,30 @@ static RegisterPrimOp primop_deepSeq({
     .fun = prim_deepSeq,
 });
 
+/* Evaluate the argument, then run memory garbage collection. */
+static void prim_gcAfter(EvalState & state, const Pos & pos, Value * * args, Value & v)
+{
+    state.forceValue(*args[0], pos);
+    v = *args[0];
+    if (GC_is_disabled()) {
+        warn("gc is disabled; ignoring __gcAfter request");
+    }
+    else {
+        printInfo("__gcAfter: forcing gc");
+        GC_gcollect();
+    }
+}
+
+static RegisterPrimOp primop_gcAfter({
+    .name = "__gcAfter",
+    .args = {"e"},
+    .doc = R"(
+      Evaluate *e*, run memory garbage collection and return the value of *e*.
+      This is only useful for testing, because garbage collection is automatic.
+    )",
+    .fun = prim_gcAfter,
+});
+
 /* Evaluate the first expression and print it on standard error.  Then
    return the second expression.  Useful for debugging. */
 static void prim_trace(EvalState & state, const Pos & pos, Value * * args, Value & v)