diff options
| -rw-r--r-- | boehmgc-coroutine-sp-fallback.diff | 22 | ||||
| -rw-r--r-- | src/libexpr/eval.cc | 25 | ||||
| -rw-r--r-- | src/libexpr/tests/coro-gc.cc | 147 | ||||
| -rw-r--r-- | src/libexpr/tests/local.mk | 2 | ||||
| -rw-r--r-- | src/libutil/serialise.cc | 39 | ||||
| -rw-r--r-- | src/libutil/serialise.hh | 6 |
6 files changed, 9 insertions, 232 deletions
diff --git a/boehmgc-coroutine-sp-fallback.diff b/boehmgc-coroutine-sp-fallback.diff index 5066d8278..2826486fb 100644 --- a/boehmgc-coroutine-sp-fallback.diff +++ b/boehmgc-coroutine-sp-fallback.diff @@ -1,8 +1,8 @@ diff --git a/darwin_stop_world.c b/darwin_stop_world.c -index 0468aaec..b348d869 100644 +index 3dbaa3fb..36a1d1f7 100644 --- a/darwin_stop_world.c +++ b/darwin_stop_world.c -@@ -356,6 +356,7 @@ GC_INNER void GC_push_all_stacks(void) +@@ -352,6 +352,7 @@ GC_INNER void GC_push_all_stacks(void) int nthreads = 0; word total_size = 0; mach_msg_type_number_t listcount = (mach_msg_type_number_t)THREAD_TABLE_SZ; @@ -10,7 +10,7 @@ index 0468aaec..b348d869 100644 if (!EXPECT(GC_thr_initialized, TRUE)) GC_thr_init(); -@@ -411,6 +412,19 @@ GC_INNER void GC_push_all_stacks(void) +@@ -407,6 +408,19 @@ GC_INNER void GC_push_all_stacks(void) GC_push_all_stack_sections(lo, hi, p->traced_stack_sect); } if (altstack_lo) { @@ -30,22 +30,6 @@ index 0468aaec..b348d869 100644 total_size += altstack_hi - altstack_lo; GC_push_all_stack(altstack_lo, altstack_hi); } -diff --git a/include/gc.h b/include/gc.h -index edab6c22..f2c61282 100644 ---- a/include/gc.h -+++ b/include/gc.h -@@ -2172,6 +2172,11 @@ GC_API void GC_CALL GC_win32_free_heap(void); - (*GC_amiga_allocwrapper_do)(a,GC_malloc_atomic_ignore_off_page) - #endif /* _AMIGA && !GC_AMIGA_MAKINGLIB */ - -+#if !__APPLE__ -+/* Patch doesn't work on apple */ -+#define NIX_BOEHM_PATCH_VERSION 1 -+#endif -+ - #ifdef __cplusplus - } /* extern "C" */ - #endif diff --git a/pthread_stop_world.c b/pthread_stop_world.c index b5d71e62..aed7b0bf 100644 --- a/pthread_stop_world.c diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 3e8857fc8..2721b6733 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -325,22 +325,6 @@ static Symbol getName(const AttrName & name, EvalState & state, Env & env) } } -#if HAVE_BOEHMGC -/* Disable GC while this object lives. Used by CoroutineContext. - * - * Boehm keeps a count of GC_disable() and GC_enable() calls, - * and only enables GC when the count matches. - */ -class BoehmDisableGC { -public: - BoehmDisableGC() { - GC_disable(); - }; - ~BoehmDisableGC() { - GC_enable(); - }; -}; -#endif static bool gcInitialised = false; @@ -365,15 +349,6 @@ void initGC() StackAllocator::defaultAllocator = &boehmGCStackAllocator; - -#if NIX_BOEHM_PATCH_VERSION != 1 - printTalkative("Unpatched BoehmGC, disabling GC inside coroutines"); - /* Used to disable GC when entering coroutines on macOS */ - create_coro_gc_hook = []() -> std::shared_ptr<void> { - return std::make_shared<BoehmDisableGC>(); - }; -#endif - /* Set the initial heap size to something fairly big (25% of physical RAM, up to a maximum of 384 MiB) so that in most cases we don't need to garbage collect at all. (Collection has a diff --git a/src/libexpr/tests/coro-gc.cc b/src/libexpr/tests/coro-gc.cc deleted file mode 100644 index f848bc2f0..000000000 --- a/src/libexpr/tests/coro-gc.cc +++ /dev/null @@ -1,147 +0,0 @@ -#include <gtest/gtest.h> -#if HAVE_BOEHMGC -#include <gc/gc.h> - -#include "eval.hh" -#include "serialise.hh" - -#endif - - -namespace nix { -#if HAVE_BOEHMGC - - static void finalizer(void *obj, void *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() { - 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); - GC_gcollect(); - GC_invoke_finalizers(); - - ASSERT_TRUE(GC_is_disabled() || *do_collect_witness); - ASSERT_FALSE(*dont_collect_witness); - ASSERT_NE(nullptr, dont_collect); - } - - TEST(CoroGC, BasicFinalizers) { - initGC(); - testFinalizerCalls(); - } - - // Run testFinalizerCalls inside a coroutine - // this tests that GC works as expected inside a coroutine - TEST(CoroGC, CoroFinalizers) { - initGC(); - - auto source = sinkToSource([&](Sink& sink) { - testFinalizerCalls(); - - // 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 - - // 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 - writeString("bar", sink); - }); - - // pass control to coroutine - std::string foo = readString(*source); - ASSERT_EQ(foo, "foo"); - - ASSERT_FALSE(GC_is_disabled()); - GC_gcollect(); - GC_invoke_finalizers(); - - // pass control to coroutine - std::string bar = readString(*source); - ASSERT_EQ(bar, "bar"); - - ASSERT_FALSE(*dont_collect_witness); - ASSERT_TRUE(*do_collect_witness); - ASSERT_NE(nullptr, dont_collect); - } -#endif -} diff --git a/src/libexpr/tests/local.mk b/src/libexpr/tests/local.mk index c36d21bfe..3e5504f71 100644 --- a/src/libexpr/tests/local.mk +++ b/src/libexpr/tests/local.mk @@ -16,4 +16,4 @@ libexpr-tests_CXXFLAGS += -I src/libexpr -I src/libutil -I src/libstore -I src/l libexpr-tests_LIBS = libstore-tests libutils-tests libexpr libutil libstore libfetchers -libexpr-tests_LDFLAGS := $(GTEST_LIBS) -lgmock -lboost_context +libexpr-tests_LDFLAGS := $(GTEST_LIBS) -lgmock diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc index 6e53239f5..7476e6f6c 100644 --- a/src/libutil/serialise.cc +++ b/src/libutil/serialise.cc @@ -186,22 +186,6 @@ static DefaultStackAllocator defaultAllocatorSingleton; StackAllocator *StackAllocator::defaultAllocator = &defaultAllocatorSingleton; -std::shared_ptr<void> (*create_coro_gc_hook)() = []() -> std::shared_ptr<void> { - return {}; -}; - -/* This class is used for entry and exit hooks on coroutines */ -class CoroutineContext { - /* Disable GC when entering the coroutine without the boehm patch, - * since it doesn't find the main thread stack in this case. - * std::shared_ptr<void> performs type-erasure, so it will call the right - * deleter. */ - const std::shared_ptr<void> coro_gc_hook = create_coro_gc_hook(); -public: - CoroutineContext() {}; - ~CoroutineContext() {}; -}; - std::unique_ptr<FinishSink> sourceToSink(std::function<void(Source &)> fun) { struct SourceToSink : FinishSink @@ -222,8 +206,7 @@ std::unique_ptr<FinishSink> sourceToSink(std::function<void(Source &)> fun) if (in.empty()) return; cur = in; - if (!coro) { - CoroutineContext ctx; + if (!coro) coro = coro_t::push_type(VirtualStackAllocator{}, [&](coro_t::pull_type & yield) { LambdaSource source([&](char *out, size_t out_len) { if (cur.empty()) { @@ -240,24 +223,17 @@ std::unique_ptr<FinishSink> sourceToSink(std::function<void(Source &)> fun) }); fun(source); }); - } if (!*coro) { abort(); } - if (!cur.empty()) { - CoroutineContext ctx; - (*coro)(false); - } + if (!cur.empty()) (*coro)(false); } void finish() override { if (!coro) return; if (!*coro) abort(); - { - CoroutineContext ctx; - (*coro)(true); - } + (*coro)(true); if (*coro) abort(); } }; @@ -288,23 +264,18 @@ std::unique_ptr<Source> sinkToSource( size_t read(char * data, size_t len) override { - if (!coro) { - CoroutineContext ctx; + if (!coro) coro = coro_t::pull_type(VirtualStackAllocator{}, [&](coro_t::push_type & yield) { LambdaSink sink([&](std::string_view data) { if (!data.empty()) yield(std::string(data)); }); fun(sink); }); - } if (!*coro) { eof(); abort(); } if (pos == cur.size()) { - if (!cur.empty()) { - CoroutineContext ctx; - (*coro)(); - } + if (!cur.empty()) (*coro)(); cur = coro->get(); pos = 0; } diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh index e99c5fcc7..7da5b07fd 100644 --- a/src/libutil/serialise.hh +++ b/src/libutil/serialise.hh @@ -501,10 +501,4 @@ struct StackAllocator { static StackAllocator *defaultAllocator; }; -/* Disabling GC when entering a coroutine (without the boehm patch). - mutable to avoid boehm gc dependency in libutil. - */ -extern std::shared_ptr<void> (*create_coro_gc_hook)(); - - } |