tests/coro-gc: refactor and split into 3 tests

src/libexpr/tests/coro-gc.cc

@@ -1,89 +1,126 @@
 #include <gtest/gtest.h>
 #if HAVE_BOEHMGC
 #include <gc/gc.h>
-#endif
 
 #include "eval.hh"
 #include "serialise.hh"
 
-
-#define guard_gc(x) GC_register_finalizer((void*)x, finalizer, x##_collected, nullptr, nullptr)
+#endif
 
 
 namespace nix {
 #if HAVE_BOEHMGC
-    static bool* uncollectable_bool() {
-        bool* res = (bool*)GC_MALLOC_UNCOLLECTABLE(1);
-        *res = false;
-        return res;
-    }
 
     static void finalizer(void *obj, void *data) {
-        //printf("finalizer: obj %p data %p\n", obj, data);
         *((bool*)data) = true;
     }
 
+    static bool* make_witness(volatile void* obj) {
+        /* We can't store the witnesses on the stack,
+           since they might be collected long afterwards */
+        bool* res = (bool*)GC_MALLOC_UNCOLLECTABLE(1);
+        *res = false;
+        GC_register_finalizer((void*)obj, finalizer, res, nullptr, nullptr);
+        return res;
+    }
+
     // Generate 2 objects, discard one, run gc,
     // see if one got collected and the other didn't
     // GC is disabled inside coroutines on __APPLE__
     static void testFinalizerCalls() {
-        bool* do_collect_collected = uncollectable_bool();
-        bool* dont_collect_collected = uncollectable_bool();
-        {
-            volatile void* do_collect = GC_MALLOC_ATOMIC(128);
-            guard_gc(do_collect);
-        }
+        volatile void* do_collect = GC_MALLOC_ATOMIC(128);
         volatile void* dont_collect = GC_MALLOC_ATOMIC(128);
-        guard_gc(dont_collect);
+
+        bool* do_collect_witness = make_witness(do_collect);
+        bool* dont_collect_witness = make_witness(dont_collect);
         GC_gcollect();
         GC_invoke_finalizers();
 
-#if !__APPLE__
-        ASSERT_TRUE(*do_collect_collected);
-#endif
-        ASSERT_FALSE(*dont_collect_collected);
+        ASSERT_TRUE(GC_is_disabled() || *do_collect_witness);
+        ASSERT_FALSE(*dont_collect_witness);
         ASSERT_NE(nullptr, dont_collect);
     }
 
-    // This test tests that boehm handles coroutine stacks correctly
-    TEST(CoroGC, CoroutineStackNotGCd) {
+    TEST(CoroGC, BasicFinalizers) {
         initGC();
         testFinalizerCalls();
+    }
 
-        bool* dont_collect_collected = uncollectable_bool();
-        bool* do_collect_collected = uncollectable_bool();
-
-        volatile void* dont_collect = GC_MALLOC_ATOMIC(128);
-        guard_gc(dont_collect);
-        {
-            volatile void* do_collect = GC_MALLOC_ATOMIC(128);
-            guard_gc(do_collect);
-        }
+    // Run testFinalizerCalls inside a coroutine
+    // this tests that GC works as expected inside a coroutine
+    TEST(CoroGC, CoroFinalizers) {
+        initGC();
 
         auto source = sinkToSource([&](Sink& sink) {
-
-#if __APPLE__
-            ASSERT_TRUE(GC_is_disabled());
-#endif
             testFinalizerCalls();
 
-            bool* dont_collect_inner_collected = uncollectable_bool();
-            bool* do_collect_inner_collected = uncollectable_bool();
-
-            volatile void* dont_collect_inner = GC_MALLOC_ATOMIC(128);
-            guard_gc(dont_collect_inner);
-            {
-                volatile void* do_collect_inner = GC_MALLOC_ATOMIC(128);
-                guard_gc(do_collect_inner);
-            }
             // pass control to main
             writeString("foo", sink);
+        });
+
+        // pass control to coroutine
+        std::string foo = readString(*source);
+        ASSERT_EQ(foo, "foo");
+    }
+
 #if __APPLE__
+    // This test tests that GC is disabled on darwin
+    // to work around the patch not being sufficient there,
+    // causing crashes whenever gc is invoked inside a coroutine
+    TEST(CoroGC, AppleCoroDisablesGC) {
+        initGC();
+        auto source = sinkToSource([&](Sink& sink) {
             ASSERT_TRUE(GC_is_disabled());
+            // pass control to main
+            writeString("foo", sink);
+
+            ASSERT_TRUE(GC_is_disabled());
+
+            // pass control to main
+            writeString("bar", sink);
+        });
+
+        // pass control to coroutine
+        std::string foo = readString(*source);
+        ASSERT_EQ(foo, "foo");
+        ASSERT_FALSE(GC_is_disabled());
+        // pass control to coroutine
+        std::string bar = readString(*source);
+        ASSERT_EQ(bar, "bar");
+
+        ASSERT_FALSE(GC_is_disabled());
+    }
 #endif
 
-            ASSERT_TRUE(*do_collect_inner_collected);
-            ASSERT_FALSE(*dont_collect_inner_collected);
+    // This test tests that boehm handles coroutine stacks correctly
+    // This test tests that coroutine stacks are registered to the GC,
+    // even when the coroutine is not running. It also tests that
+    // the main stack is still registered to the GC when the coroutine is running.
+    TEST(CoroGC, CoroutineStackNotGCd) {
+        initGC();
+
+        volatile void* do_collect = GC_MALLOC_ATOMIC(128);
+        volatile void* dont_collect = GC_MALLOC_ATOMIC(128);
+
+        bool* do_collect_witness = make_witness(do_collect);
+        bool* dont_collect_witness = make_witness(dont_collect);
+
+        do_collect = nullptr;
+
+        auto source = sinkToSource([&](Sink& sink) {
+            volatile void* dont_collect_inner = GC_MALLOC_ATOMIC(128);
+            volatile void* do_collect_inner = GC_MALLOC_ATOMIC(128);
+
+            bool* do_collect_inner_witness = make_witness(do_collect_inner);
+            bool* dont_collect_inner_witness = make_witness(dont_collect_inner);
+
+            do_collect_inner = nullptr;
+
+            // pass control to main
+            writeString("foo", sink);
+
+            ASSERT_FALSE(*dont_collect_inner_witness);
+            ASSERT_TRUE(*do_collect_inner_witness);
             ASSERT_NE(nullptr, dont_collect_inner);
 
             // pass control to main
@@ -102,8 +139,8 @@ namespace nix {
         std::string bar = readString(*source);
         ASSERT_EQ(bar, "bar");
 
-        ASSERT_FALSE(*dont_collect_collected);
-        ASSERT_TRUE(*do_collect_collected);
+        ASSERT_FALSE(*dont_collect_witness);
+        ASSERT_TRUE(*do_collect_witness);
         ASSERT_NE(nullptr, dont_collect);
     }
 #endif