Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 2.13-maintenance] disable gc on coroutine #8181

Merged
merged 4 commits into from
Apr 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions boehmgc-coroutine-sp-fallback.diff
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
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;
+ size_t stack_limit;
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) {
Expand All @@ -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
Expand Down
25 changes: 25 additions & 0 deletions src/libexpr/eval.cc
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,22 @@ 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;

Expand All @@ -347,6 +363,15 @@ 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
Expand Down
39 changes: 34 additions & 5 deletions src/libutil/serialise.cc
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,22 @@ 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
Expand All @@ -206,7 +222,8 @@ std::unique_ptr<FinishSink> sourceToSink(std::function<void(Source &)> fun)
if (in.empty()) return;
cur = in;

if (!coro)
if (!coro) {
CoroutineContext ctx;
coro = coro_t::push_type(VirtualStackAllocator{}, [&](coro_t::pull_type & yield) {
LambdaSource source([&](char *out, size_t out_len) {
if (cur.empty()) {
Expand All @@ -223,17 +240,24 @@ std::unique_ptr<FinishSink> sourceToSink(std::function<void(Source &)> fun)
});
fun(source);
});
}

if (!*coro) { abort(); }

if (!cur.empty()) (*coro)(false);
if (!cur.empty()) {
CoroutineContext ctx;
(*coro)(false);
}
}

void finish() override
{
if (!coro) return;
if (!*coro) abort();
(*coro)(true);
{
CoroutineContext ctx;
(*coro)(true);
}
if (*coro) abort();
}
};
Expand Down Expand Up @@ -264,18 +288,23 @@ std::unique_ptr<Source> sinkToSource(

size_t read(char * data, size_t len) override
{
if (!coro)
if (!coro) {
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));
});
fun(sink);
});
}

if (!*coro) { eof(); abort(); }

if (pos == cur.size()) {
if (!cur.empty()) (*coro)();
if (!cur.empty()) {
CoroutineContext ctx;
(*coro)();
}
cur = coro->get();
pos = 0;
}
Expand Down
6 changes: 6 additions & 0 deletions src/libutil/serialise.hh
Original file line number Diff line number Diff line change
Expand Up @@ -501,4 +501,10 @@ 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)();


}