mirror of
https://github.com/ruby/ruby.git
synced 2025-08-15 13:39:04 +02:00
Get String#crypt working with multi-ractor in cases where !HAVE_CRYPT_R
In commit 12f7ba5ed4
, 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>
This commit is contained in:
parent
a1996b32a9
commit
328e3029d8
1 changed files with 14 additions and 6 deletions
20
string.c
20
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;
|
||||
}
|
||||
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue