From 5184b40dd4dc446660cd35c3e53896324e95b317 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Fri, 8 Sep 2023 00:32:54 +1200 Subject: [PATCH] Extract `do_mutex_lock_check_interrupts` to try and fix `ppc64le`. (#8393) We found some tests were hanging in `do_mutex_lock`, specifically the fiber scheduler autoload test. After much investigation, it may be a code generation bug. Because we didn't change the code, but only extracted it into a separate function, and it appears to fix the problem. --- thread_sync.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/thread_sync.c b/thread_sync.c index 85ebec4d8c..63f0228d9d 100644 --- a/thread_sync.c +++ b/thread_sync.c @@ -290,6 +290,28 @@ delete_from_waitq(VALUE value) return Qnil; } +static void +do_mutex_lock_check_interrupts(int interruptible_p, rb_fiber_t *fiber, rb_mutex_t *mutex, rb_thread_t *thread) +{ + // We extracted this function because we suspect there can be a codegen bug + // on ppc64le and moving this code to a separate function seems to fix the + // problem, at least in my tests. + if (interruptible_p) { + /* release mutex before checking for interrupts...as interrupt checking + * code might call rb_raise() */ + if (mutex->fiber == fiber) { + mutex->fiber = 0; + } + + RUBY_VM_CHECK_INTS_BLOCKING(thread->ec); /* may release mutex */ + + if (!mutex->fiber) { + mutex->fiber = fiber; + } + } +} + + static VALUE do_mutex_lock(VALUE self, int interruptible_p) { @@ -378,15 +400,7 @@ do_mutex_lock(VALUE self, int interruptible_p) rb_ractor_sleeper_threads_dec(th->ractor); } - if (interruptible_p) { - /* release mutex before checking for interrupts...as interrupt checking - * code might call rb_raise() */ - if (mutex->fiber == fiber) mutex->fiber = 0; - RUBY_VM_CHECK_INTS_BLOCKING(th->ec); /* may release mutex */ - if (!mutex->fiber) { - mutex->fiber = fiber; - } - } + do_mutex_lock_check_interrupts(interruptible_p, fiber, mutex, th); } if (mutex->fiber == fiber) mutex_locked(th, self);