diff options
author | KJ Tsanaktsidis <kj@kjtsanaktsidis.id.au> | 2024-07-12 15:13:22 +1000 |
---|---|---|
committer | KJ Tsanaktsidis <kj@kjtsanaktsidis.id.au> | 2024-07-19 13:44:01 +1000 |
commit | ca0dae25ed51627c411dfdbe9dec3901a321bff9 (patch) | |
tree | fd21f08e89855c3e600cea1d5b57cfdfd3e890ef | |
parent | e5c06005f182a980637069659843e89f48f5566c (diff) | |
download | ruby-ca0dae25ed51627c411dfdbe9dec3901a321bff9.tar.gz |
Don't crash if madvise(MADV_FREE or MADV_DONTNEED) fails
The M:N threading stack cleanup machinery tries to call MADV_FREE on the native
thread's stack, and calls rb_bug if it fails. Unfortunately, there's no way to
distinguish between "You passed bad parameters to madvise" and "MADV_FREE is
not supported on the kernel you are running on"; both cases just return EINVAL.
This means that if you have a Ruby on a system that was built on a system with
MADV_FREE and run it on a system without it, you get a crash in nt_free_stack.
I ran into this because rr actually emulates MADV_FREE by just returning EINVAL
and pretending it's not supported (since it can otherwise introduce
nondeterministic behaviour). So if you run bootstraptest/test_ractor.rb under
rr, you get this crash.
I think we should just get rid of the error handling here; freeing memory like
this is strictly optional anyway.
[Bug #20632]
-rw-r--r-- | thread_pthread_mn.c | 35 |
1 files changed, 26 insertions, 9 deletions
diff --git a/thread_pthread_mn.c b/thread_pthread_mn.c index 6b0631ad5a..08d587e2b2 100644 --- a/thread_pthread_mn.c +++ b/thread_pthread_mn.c @@ -338,6 +338,30 @@ nt_alloc_stack(rb_vm_t *vm, void **vm_stack, void **machine_stack) } static void +nt_madvise_free_or_dontneed(void *addr, size_t len) +{ + /* There is no real way to perform error handling here. Both MADV_FREE + * and MADV_DONTNEED are both documented to pretty much only return EINVAL + * for a huge variety of errors. It's indistinguishable if madvise fails + * because the parameters were bad, or because the kernel we're running on + * does not support the given advice. This kind of free-but-don't-unmap + * is best-effort anyway, so don't sweat it. + * + * n.b. A very common case of "the kernel doesn't support MADV_FREE and + * returns EINVAL" is running under the `rr` debugger; it makes all + * MADV_FREE calls return EINVAL. */ + +#if defined(MADV_FREE) + int r = madvise(addr, len, MADV_FREE); + // Return on success, or else try MADV_DONTNEED + if (r == 0) return; +#endif +#if defined(MADV_DONTNEED) + madvise(addr, len, MADV_DONTNEED); +#endif +} + +static void nt_free_stack(void *mstack) { if (!mstack) return; @@ -358,19 +382,12 @@ nt_free_stack(void *mstack) ch->free_stack[ch->free_stack_pos++] = idx; // clear the stack pages -#if defined(MADV_FREE) - int r = madvise(stack, nt_thread_stack_size(), MADV_FREE); -#elif defined(MADV_DONTNEED) - int r = madvise(stack, nt_thread_stack_size(), MADV_DONTNEED); -#else - int r = 0; -#endif - - if (r != 0) rb_bug("madvise errno:%d", errno); + nt_madvise_free_or_dontneed(stack, nt_thread_stack_size()); } rb_native_mutex_unlock(&nt_machine_stack_lock); } + static int native_thread_check_and_create_shared(rb_vm_t *vm) { |