From 0fd8f542a8ddb303f589ff6ca3343f36e3a783c0 Mon Sep 17 00:00:00 2001 From: Yorick van Pelt Date: Tue, 31 Jan 2023 17:54:35 +0100 Subject: [PATCH 1/6] tests/coro-gc: create test for boehm stack patch Regression test for #7679 --- src/libexpr/tests/coro-gc.cc | 99 ++++++++++++++++++++++++++++++++++++ src/libexpr/tests/local.mk | 2 +- 2 files changed, 100 insertions(+), 1 deletion(-) create mode 100644 src/libexpr/tests/coro-gc.cc diff --git a/src/libexpr/tests/coro-gc.cc b/src/libexpr/tests/coro-gc.cc new file mode 100644 index 000000000..6023b9b5e --- /dev/null +++ b/src/libexpr/tests/coro-gc.cc @@ -0,0 +1,99 @@ +#include +#if HAVE_BOEHMGC +#include +#endif + +#include "eval.hh" +#include "serialise.hh" + + +#define guard_gc(x) GC_register_finalizer((void*)x, finalizer, x##_collected, nullptr, nullptr) + + +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; + } + + // Generate 2 objects, discard one, run gc, + // see if one got collected and the other didn't + 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* dont_collect = GC_MALLOC_ATOMIC(128); + guard_gc(dont_collect); + GC_gcollect(); + GC_invoke_finalizers(); + + ASSERT_TRUE(*do_collect_collected); + ASSERT_FALSE(*dont_collect_collected); + ASSERT_NE(nullptr, dont_collect); + } + + // This test tests that boehm handles coroutine stacks correctly + TEST(CoroGC, CoroutineStackNotGCd) { + 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); + } + + auto source = sinkToSource([&](Sink& sink) { + 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); + + ASSERT_TRUE(*do_collect_inner_collected); + ASSERT_FALSE(*dont_collect_inner_collected); + 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"); + + GC_gcollect(); + GC_invoke_finalizers(); + + // pass control to coroutine + std::string bar = readString(*source); + ASSERT_EQ(bar, "bar"); + + ASSERT_FALSE(*dont_collect_collected); + ASSERT_TRUE(*do_collect_collected); + ASSERT_NE(nullptr, dont_collect); + } +#endif +} diff --git a/src/libexpr/tests/local.mk b/src/libexpr/tests/local.mk index 3e5504f71..c36d21bfe 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 +libexpr-tests_LDFLAGS := $(GTEST_LIBS) -lgmock -lboost_context From eaeb994d8b9d2152e076897bf430c8ac205d3d1a Mon Sep 17 00:00:00 2001 From: Yorick van Pelt Date: Fri, 3 Feb 2023 17:50:01 +0100 Subject: [PATCH 2/6] Disable GC inside coroutines on mac OS --- src/libexpr/eval.cc | 20 +++++++++++++++++++ src/libexpr/tests/coro-gc.cc | 11 +++++++++++ src/libutil/serialise.cc | 38 +++++++++++++++++++++++++++++++----- src/libutil/serialise.hh | 10 ++++++++++ 4 files changed, 74 insertions(+), 5 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 3e37c7f60..a5a198926 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -326,6 +326,20 @@ static Symbol getName(const AttrName & name, EvalState & state, Env & env) } +class BoehmDisableGC : public DisableGC { +public: + BoehmDisableGC() { +#if HAVE_BOEHMGC + GC_disable(); +#endif + }; + virtual ~BoehmDisableGC() override { +#if HAVE_BOEHMGC + GC_enable(); +#endif + }; +}; + static bool gcInitialised = false; void initGC() @@ -349,6 +363,12 @@ void initGC() StackAllocator::defaultAllocator = &boehmGCStackAllocator; + + /* Used to disable GC when entering coroutines on macOS */ + DisableGC::create = []() { + return std::dynamic_pointer_cast(std::make_shared()); + }; + /* 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 index 6023b9b5e..8d9585cc7 100644 --- a/src/libexpr/tests/coro-gc.cc +++ b/src/libexpr/tests/coro-gc.cc @@ -25,6 +25,7 @@ namespace nix { // 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(); @@ -37,7 +38,9 @@ namespace nix { GC_gcollect(); GC_invoke_finalizers(); +#if !__APPLE__ ASSERT_TRUE(*do_collect_collected); +#endif ASSERT_FALSE(*dont_collect_collected); ASSERT_NE(nullptr, dont_collect); } @@ -58,6 +61,10 @@ namespace nix { } auto source = sinkToSource([&](Sink& sink) { + +#if __APPLE__ + ASSERT_TRUE(GC_is_disabled()); +#endif testFinalizerCalls(); bool* dont_collect_inner_collected = uncollectable_bool(); @@ -71,6 +78,9 @@ namespace nix { } // pass control to main writeString("foo", sink); +#if __APPLE__ + ASSERT_TRUE(GC_is_disabled()); +#endif ASSERT_TRUE(*do_collect_inner_collected); ASSERT_FALSE(*dont_collect_inner_collected); @@ -84,6 +94,7 @@ namespace nix { std::string foo = readString(*source); ASSERT_EQ(foo, "foo"); + ASSERT_FALSE(GC_is_disabled()); GC_gcollect(); GC_invoke_finalizers(); diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc index c653db9d0..531100c01 100644 --- a/src/libutil/serialise.cc +++ b/src/libutil/serialise.cc @@ -186,6 +186,10 @@ static DefaultStackAllocator defaultAllocatorSingleton; StackAllocator *StackAllocator::defaultAllocator = &defaultAllocatorSingleton; +std::shared_ptr (*DisableGC::create)() = []() { + return std::dynamic_pointer_cast(std::make_shared()); +}; + std::unique_ptr sourceToSink(std::function fun) { struct SourceToSink : FinishSink @@ -206,7 +210,11 @@ std::unique_ptr sourceToSink(std::function fun) if (in.empty()) return; cur = in; - if (!coro) + if (!coro) { +#if __APPLE__ + /* Disable GC when entering the coroutine on macOS, since it doesn't find the main thread stack in this case */ + auto disablegc = DisableGC::create(); +#endif coro = coro_t::push_type(VirtualStackAllocator{}, [&](coro_t::pull_type & yield) { LambdaSource source([&](char *out, size_t out_len) { if (cur.empty()) { @@ -223,17 +231,28 @@ std::unique_ptr sourceToSink(std::function fun) }); fun(source); }); + } if (!*coro) { abort(); } - if (!cur.empty()) (*coro)(false); + if (!cur.empty()) { +#if __APPLE__ + auto disablegc = DisableGC::create(); +#endif + (*coro)(false); + } } void finish() override { if (!coro) return; if (!*coro) abort(); - (*coro)(true); + { +#if __APPLE__ + auto disablegc = DisableGC::create(); +#endif + (*coro)(true); + } if (*coro) abort(); } }; @@ -264,18 +283,27 @@ std::unique_ptr sinkToSource( size_t read(char * data, size_t len) override { - if (!coro) + if (!coro) { +#if __APPLE__ + auto disablegc = DisableGC::create(); +#endif 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()) (*coro)(); + if (!cur.empty()) { +#if __APPLE__ + auto disablegc = DisableGC::create(); +#endif + (*coro)(); + } cur = coro->get(); pos = 0; } diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh index 7da5b07fd..fb1f14a3b 100644 --- a/src/libutil/serialise.hh +++ b/src/libutil/serialise.hh @@ -501,4 +501,14 @@ struct StackAllocator { static StackAllocator *defaultAllocator; }; +/* Disabling GC when entering a coroutine (on macos). + ::create is to avoid boehm gc dependency in libutil. + */ +class DisableGC { +public: + DisableGC() {}; + virtual ~DisableGC() {}; + static std::shared_ptr (*create)(); +}; + } From 53bb4a53279170c236c8a9252297b1132620f4ea Mon Sep 17 00:00:00 2001 From: Yorick van Pelt Date: Tue, 7 Feb 2023 14:59:45 +0100 Subject: [PATCH 3/6] tests/coro-gc: refactor and split into 3 tests --- src/libexpr/tests/coro-gc.cc | 133 ++++++++++++++++++++++------------- 1 file changed, 85 insertions(+), 48 deletions(-) diff --git a/src/libexpr/tests/coro-gc.cc b/src/libexpr/tests/coro-gc.cc index 8d9585cc7..f848bc2f0 100644 --- a/src/libexpr/tests/coro-gc.cc +++ b/src/libexpr/tests/coro-gc.cc @@ -1,89 +1,126 @@ #include #if HAVE_BOEHMGC #include -#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 From 4c73eab92351e0942dc27f84fec1bf8c94619d02 Mon Sep 17 00:00:00 2001 From: Yorick van Pelt Date: Fri, 10 Feb 2023 15:04:17 +0100 Subject: [PATCH 4/6] DisableGC: replace by CoroutineContext, std::shared_ptr --- src/libexpr/eval.cc | 20 +++++++++++--------- src/libutil/serialise.cc | 38 ++++++++++++++++++++------------------ src/libutil/serialise.hh | 10 +++------- 3 files changed, 34 insertions(+), 34 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index a5a198926..01680afff 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -325,20 +325,22 @@ static Symbol getName(const AttrName & name, EvalState & state, Env & env) } } - -class BoehmDisableGC : public DisableGC { +#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() { -#if HAVE_BOEHMGC GC_disable(); -#endif }; - virtual ~BoehmDisableGC() override { -#if HAVE_BOEHMGC + ~BoehmDisableGC() { GC_enable(); -#endif }; }; +#endif static bool gcInitialised = false; @@ -365,8 +367,8 @@ void initGC() /* Used to disable GC when entering coroutines on macOS */ - DisableGC::create = []() { - return std::dynamic_pointer_cast(std::make_shared()); + create_disable_gc = []() -> std::shared_ptr { + return std::make_shared(); }; /* Set the initial heap size to something fairly big (25% of diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc index 531100c01..7ef24d458 100644 --- a/src/libutil/serialise.cc +++ b/src/libutil/serialise.cc @@ -186,8 +186,21 @@ static DefaultStackAllocator defaultAllocatorSingleton; StackAllocator *StackAllocator::defaultAllocator = &defaultAllocatorSingleton; -std::shared_ptr (*DisableGC::create)() = []() { - return std::dynamic_pointer_cast(std::make_shared()); +std::shared_ptr (*create_disable_gc)() = []() -> std::shared_ptr { + return {}; +}; + +/* This class is used for entry and exit hooks on coroutines */ +class CoroutineContext { +#if __APPLE__ + /* Disable GC when entering the coroutine on macOS, since it doesn't find the main thread stack in this case. + * std::shared_ptr performs type-erasure, so it will call the right + * deleter. */ + const std::shared_ptr disable_gc = create_disable_gc(); +#endif +public: + CoroutineContext() {}; + ~CoroutineContext() {}; }; std::unique_ptr sourceToSink(std::function fun) @@ -211,10 +224,7 @@ std::unique_ptr sourceToSink(std::function fun) cur = in; if (!coro) { -#if __APPLE__ - /* Disable GC when entering the coroutine on macOS, since it doesn't find the main thread stack in this case */ - auto disablegc = DisableGC::create(); -#endif + CoroutineContext ctx; coro = coro_t::push_type(VirtualStackAllocator{}, [&](coro_t::pull_type & yield) { LambdaSource source([&](char *out, size_t out_len) { if (cur.empty()) { @@ -236,9 +246,7 @@ std::unique_ptr sourceToSink(std::function fun) if (!*coro) { abort(); } if (!cur.empty()) { -#if __APPLE__ - auto disablegc = DisableGC::create(); -#endif + CoroutineContext ctx; (*coro)(false); } } @@ -248,9 +256,7 @@ std::unique_ptr sourceToSink(std::function fun) if (!coro) return; if (!*coro) abort(); { -#if __APPLE__ - auto disablegc = DisableGC::create(); -#endif + CoroutineContext ctx; (*coro)(true); } if (*coro) abort(); @@ -284,9 +290,7 @@ std::unique_ptr sinkToSource( size_t read(char * data, size_t len) override { if (!coro) { -#if __APPLE__ - auto disablegc = DisableGC::create(); -#endif + CoroutineContext ctx; coro = coro_t::pull_type(VirtualStackAllocator{}, [&](coro_t::push_type & yield) { LambdaSink sink([&](std::string_view data) { if (!data.empty()) yield(std::string(data)); @@ -299,9 +303,7 @@ std::unique_ptr sinkToSource( if (pos == cur.size()) { if (!cur.empty()) { -#if __APPLE__ - auto disablegc = DisableGC::create(); -#endif + CoroutineContext ctx; (*coro)(); } cur = coro->get(); diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh index fb1f14a3b..58b9499db 100644 --- a/src/libutil/serialise.hh +++ b/src/libutil/serialise.hh @@ -502,13 +502,9 @@ struct StackAllocator { }; /* Disabling GC when entering a coroutine (on macos). - ::create is to avoid boehm gc dependency in libutil. + mutable to avoid boehm gc dependency in libutil. */ -class DisableGC { -public: - DisableGC() {}; - virtual ~DisableGC() {}; - static std::shared_ptr (*create)(); -}; +extern std::shared_ptr (*create_disable_gc)(); + } From 176005749cbc413514cc7fca31587de184720b62 Mon Sep 17 00:00:00 2001 From: Yorick van Pelt Date: Wed, 1 Mar 2023 15:07:00 +0100 Subject: [PATCH 5/6] Always disable GC in a coroutine unless the patch is applied --- boehmgc-coroutine-sp-fallback.diff | 22 +++++++++++++++++++--- src/libexpr/eval.cc | 4 +++- src/libutil/serialise.cc | 9 ++++----- src/libutil/serialise.hh | 4 ++-- 4 files changed, 28 insertions(+), 11 deletions(-) diff --git a/boehmgc-coroutine-sp-fallback.diff b/boehmgc-coroutine-sp-fallback.diff index 2826486fb..5066d8278 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 3dbaa3fb..36a1d1f7 100644 +index 0468aaec..b348d869 100644 --- a/darwin_stop_world.c +++ b/darwin_stop_world.c -@@ -352,6 +352,7 @@ GC_INNER void GC_push_all_stacks(void) +@@ -356,6 +356,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 3dbaa3fb..36a1d1f7 100644 if (!EXPECT(GC_thr_initialized, TRUE)) GC_thr_init(); -@@ -407,6 +408,19 @@ GC_INNER void GC_push_all_stacks(void) +@@ -411,6 +412,19 @@ GC_INNER void GC_push_all_stacks(void) GC_push_all_stack_sections(lo, hi, p->traced_stack_sect); } if (altstack_lo) { @@ -30,6 +30,22 @@ index 3dbaa3fb..36a1d1f7 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 01680afff..9667136e8 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -366,10 +366,12 @@ void initGC() StackAllocator::defaultAllocator = &boehmGCStackAllocator; +#if NIX_BOEHM_PATCH_VERSION != 1 /* Used to disable GC when entering coroutines on macOS */ - create_disable_gc = []() -> std::shared_ptr { + create_coro_gc_hook = []() -> std::shared_ptr { return std::make_shared(); }; +#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 diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc index 7ef24d458..f83bb2848 100644 --- a/src/libutil/serialise.cc +++ b/src/libutil/serialise.cc @@ -186,18 +186,17 @@ static DefaultStackAllocator defaultAllocatorSingleton; StackAllocator *StackAllocator::defaultAllocator = &defaultAllocatorSingleton; -std::shared_ptr (*create_disable_gc)() = []() -> std::shared_ptr { +std::shared_ptr (*create_coro_gc_hook)() = []() -> std::shared_ptr { return {}; }; /* This class is used for entry and exit hooks on coroutines */ class CoroutineContext { -#if __APPLE__ - /* Disable GC when entering the coroutine on macOS, since it doesn't find the main thread stack in this case. + /* 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 performs type-erasure, so it will call the right * deleter. */ - const std::shared_ptr disable_gc = create_disable_gc(); -#endif + const std::shared_ptr coro_gc_hook = create_coro_gc_hook(); public: CoroutineContext() {}; ~CoroutineContext() {}; diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh index 58b9499db..e99c5fcc7 100644 --- a/src/libutil/serialise.hh +++ b/src/libutil/serialise.hh @@ -501,10 +501,10 @@ struct StackAllocator { static StackAllocator *defaultAllocator; }; -/* Disabling GC when entering a coroutine (on macos). +/* Disabling GC when entering a coroutine (without the boehm patch). mutable to avoid boehm gc dependency in libutil. */ -extern std::shared_ptr (*create_disable_gc)(); +extern std::shared_ptr (*create_coro_gc_hook)(); } From 2683734936760dad87a33710d0264266aea96ca4 Mon Sep 17 00:00:00 2001 From: Yorick van Pelt Date: Fri, 3 Mar 2023 11:43:47 +0100 Subject: [PATCH 6/6] Add talkative msg for coro gc debug --- src/libexpr/eval.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 9667136e8..0d2a3c815 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -367,6 +367,7 @@ void initGC() #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 { return std::make_shared();