From eaeb994d8b9d2152e076897bf430c8ac205d3d1a Mon Sep 17 00:00:00 2001 From: Yorick van Pelt Date: Fri, 3 Feb 2023 17:50:01 +0100 Subject: [PATCH] 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)(); +}; + }