aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorKJ Tsanaktsidis <kj@kjtsanaktsidis.id.au>2024-07-12 15:13:22 +1000
committerKJ Tsanaktsidis <kj@kjtsanaktsidis.id.au>2024-07-19 13:44:01 +1000
commitca0dae25ed51627c411dfdbe9dec3901a321bff9 (patch)
treefd21f08e89855c3e600cea1d5b57cfdfd3e890ef
parente5c06005f182a980637069659843e89f48f5566c (diff)
downloadruby-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.c35
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)
{