From 328e3029d875c4c74c1d732bee7ea35d659dd608 Mon Sep 17 00:00:00 2001 From: Luke Gruber Date: Mon, 9 Jun 2025 17:22:33 -0400 Subject: [PATCH] Get String#crypt working with multi-ractor in cases where !HAVE_CRYPT_R In commit 12f7ba5ed4a, ractor safety was added to String#crypt, however in certain cases it can cause a deadlock. When we lock a native mutex, we cannot allocate ruby objects because they might trigger GC which starts a VM barrier. If the barrier is triggered and other native threads are waiting on this mutex, they will not be able to be woken up in order to join the barrier. To fix this, we don't allocate ruby objects when we hold the lock. The following could reproduce the problem: ```ruby strings = [] 10_000.times do |i| strings << "my string #{i}" end STRINGS = Ractor.make_shareable(strings) rs = [] 100.times do rs << Ractor.new do STRINGS.each do |s| s.dup.crypt(s.dup) end end end while rs.any? r, obj = Ractor.select(*rs) rs.delete(r) end ``` I will not be adding tests because I am almost finished a PR to enable running test-all test cases inside many ractors at once, which is how I found the issue. Co-authored-by: jhawthorn --- string.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/string.c b/string.c index 1ef4f03b75..8501125b02 100644 --- a/string.c +++ b/string.c @@ -11169,11 +11169,6 @@ rb_str_oct(VALUE str) static struct { rb_nativethread_lock_t lock; } crypt_mutex = {PTHREAD_MUTEX_INITIALIZER}; - -static void -crypt_mutex_initialize(void) -{ -} #endif /* @@ -11244,6 +11239,7 @@ rb_str_crypt(VALUE str, VALUE salt) struct crypt_data *data; # define CRYPT_END() ALLOCV_END(databuf) #else + char *tmp_buf; extern char *crypt(const char *, const char *); # define CRYPT_END() rb_nativethread_lock_unlock(&crypt_mutex.lock) #endif @@ -11278,7 +11274,6 @@ rb_str_crypt(VALUE str, VALUE salt) # endif res = crypt_r(s, saltp, data); #else - crypt_mutex_initialize(); rb_nativethread_lock_lock(&crypt_mutex.lock); res = crypt(s, saltp); #endif @@ -11287,8 +11282,21 @@ rb_str_crypt(VALUE str, VALUE salt) CRYPT_END(); rb_syserr_fail(err, "crypt"); } +#ifdef HAVE_CRYPT_R result = rb_str_new_cstr(res); CRYPT_END(); +#else + // We need to copy this buffer because it's static and we need to unlock the mutex + // before allocating a new object (the string to be returned). If we allocate while + // holding the lock, we could run GC which fires the VM barrier and causes a deadlock + // if other ractors are waiting on this lock. + size_t res_size = strlen(res)+1; + tmp_buf = ALLOCA_N(char, res_size); // should be small enough to alloca + memcpy(tmp_buf, res, res_size); + res = tmp_buf; + CRYPT_END(); + result = rb_str_new_cstr(res); +#endif return result; }