diff options
| author | Luke Gruber <luke.gruber@shopify.com> | 2025-06-09 17:22:33 -0400 |
|---|---|---|
| committer | John Hawthorn <john@hawthorn.email> | 2025-06-25 14:11:08 -0700 |
| commit | 328e3029d875c4c74c1d732bee7ea35d659dd608 (patch) | |
| tree | a43a792e9b524dafb9f4c16a7dcd3ef3ec633bf9 /string.c | |
| parent | a1996b32a95c12e0c1f6fd5665ba490b4245f18c (diff) | |
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 <john@hawthorn.email>
Diffstat (limited to 'string.c')
| -rw-r--r-- | string.c | 20 |
1 files changed, 14 insertions, 6 deletions
@@ -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; } |
