Skip to content

Revert "Merge pull request #13709 from NixOS/boehm-coroutines-sp" #13731

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.

Sign up for GitHub

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

Merged
Mic92 merged 1 commit into from
Aug 11, 2025
Merged

Revert "Merge pull request #13709 from NixOS/boehm-coroutines-sp" #13731

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
64 changes: 0 additions & 64 deletions src/libexpr/eval-gc.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,67 +33,6 @@ static void * oomHandler(size_t requested)
throw std::bad_alloc();
}

/**
* When a thread goes into a coroutine, we lose its original sp until
* control flow returns to the thread. This causes Boehm GC to crash
* since it will scan memory between the coroutine's sp and the
* original stack base of the thread. Therefore, we detect when the
* current sp is outside of the original thread stack and push the
* entire thread stack instead, as an approximation.
*
* This is not optimal, because it causes the stack below sp to be
* scanned. However, we usually we don't have active coroutines during
* evaluation, so this is acceptable.
*
* Note that we don't scan coroutine stacks. It's currently assumed
* that we don't have GC roots in coroutines.
*/
void fixupBoehmStackPointer(void ** sp_ptr, void * _pthread_id)
{
void *& sp = *sp_ptr;
auto pthread_id = reinterpret_cast<pthread_t>(_pthread_id);
size_t osStackSize;
// The low address of the stack, which grows down.
void * osStackLimit;

# ifdef __APPLE__
osStackSize = pthread_get_stacksize_np(pthread_id);
osStackLimit = pthread_get_stackaddr_np(pthread_id);
# else
pthread_attr_t pattr;
if (pthread_attr_init(&pattr)) {
throw Error("fixupBoehmStackPointer: pthread_attr_init failed");
}
# ifdef HAVE_PTHREAD_GETATTR_NP
if (pthread_getattr_np(pthread_id, &pattr)) {
throw Error("fixupBoehmStackPointer: pthread_getattr_np failed");
}
# elif HAVE_PTHREAD_ATTR_GET_NP
if (!pthread_attr_init(&pattr)) {
throw Error("fixupBoehmStackPointer: pthread_attr_init failed");
}
if (!pthread_attr_get_np(pthread_id, &pattr)) {
throw Error("fixupBoehmStackPointer: pthread_attr_get_np failed");
}
# else
# error "Need one of `pthread_attr_get_np` or `pthread_getattr_np`"
# endif
if (pthread_attr_getstack(&pattr, &osStackLimit, &osStackSize)) {
throw Error("fixupBoehmStackPointer: pthread_attr_getstack failed");
}
if (pthread_attr_destroy(&pattr)) {
throw Error("fixupBoehmStackPointer: pthread_attr_destroy failed");
}
# endif

void * osStackBase = (char *) osStackLimit + osStackSize;
// NOTE: We assume the stack grows down, as it does on all architectures we support.
// Architectures that grow the stack up are rare.
if (sp >= osStackBase || sp < osStackLimit) { // sp is outside the os stack
sp = osStackLimit;
}
}

static inline void initGCReal()
{
/* Initialise the Boehm garbage collector. */
Expand Down Expand Up @@ -124,9 +63,6 @@ static inline void initGCReal()

GC_set_oom_fn(oomHandler);

GC_set_sp_corrector(&fixupBoehmStackPointer);
assert(GC_get_sp_corrector());

/* 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
Loading