Skip to content

Commit 63cbe3f

Browse files
committed
Proof of Concept: Allow to prevent fork from happening in known fork unsafe API
[Feature #20590] For better of for worse, fork(2) remain the primary provider of parallelism in Ruby programs. Even though it's frowned uppon in many circles, and a lot of literature will simply state that only async-signal safe APIs are safe to use after `fork()`, in practice most APIs work well as long as you are careful about not forking while another thread is holding a pthread mutex. One of the APIs that is known cause fork safety issues is `getaddrinfo`. If you fork while another thread is inside `getaddrinfo`, a mutex may be left locked in the child, with no way to unlock it. I think we could reduce the impact of these problem by preventing in for the most notorious and common cases, by locking around `fork(2)` and known unsafe APIs with a read-write lock.
1 parent 2e5680d commit 63cbe3f

File tree

6 files changed

+82
-2
lines changed

6 files changed

+82
-2
lines changed

ext/socket/raddrinfo.c

+14-2
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,12 @@ nogvl_getaddrinfo(void *arg)
327327
return (void *)(VALUE)ret;
328328
}
329329

330+
static void *
331+
fork_safe_getaddrinfo(void *arg)
332+
{
333+
return rb_thread_prevent_fork(nogvl_getaddrinfo, arg);
334+
}
335+
330336
static int
331337
rb_getaddrinfo(const char *hostp, const char *portp, const struct addrinfo *hints, struct addrinfo **ai)
332338
{
@@ -336,7 +342,7 @@ rb_getaddrinfo(const char *hostp, const char *portp, const struct addrinfo *hint
336342
arg.service = portp;
337343
arg.hints = hints;
338344
arg.res = ai;
339-
return (int)(VALUE)rb_thread_call_without_gvl(nogvl_getaddrinfo, &arg, RUBY_UBF_IO, 0);
345+
return (int)(VALUE)rb_thread_call_without_gvl(fork_safe_getaddrinfo, &arg, RUBY_UBF_IO, 0);
340346
}
341347

342348
#elif GETADDRINFO_IMPL == 2
@@ -477,6 +483,12 @@ do_pthread_create(pthread_t *th, void *(*start_routine) (void *), void *arg)
477483
return ret;
478484
}
479485

486+
static void *
487+
fork_safe_do_getaddrinfo(void *ptr)
488+
{
489+
return rb_thread_prevent_fork(do_getaddrinfo, ptr);
490+
}
491+
480492
static int
481493
rb_getaddrinfo(const char *hostp, const char *portp, const struct addrinfo *hints, struct addrinfo **ai)
482494
{
@@ -493,7 +505,7 @@ rb_getaddrinfo(const char *hostp, const char *portp, const struct addrinfo *hint
493505
}
494506

495507
pthread_t th;
496-
if (do_pthread_create(&th, do_getaddrinfo, arg) != 0) {
508+
if (do_pthread_create(&th, fork_safe_do_getaddrinfo, arg) != 0) {
497509
int err = errno;
498510
free_getaddrinfo_arg(arg);
499511
errno = err;

internal/thread.h

+5
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ VALUE rb_thread_shield_wait(VALUE self);
4646
VALUE rb_thread_shield_release(VALUE self);
4747
VALUE rb_thread_shield_destroy(VALUE self);
4848
int rb_thread_to_be_killed(VALUE thread);
49+
void rb_thread_acquire_fork_lock(void);
50+
void rb_thread_release_fork_lock(void);
51+
void rb_thread_reset_fork_lock(void);
4952
void rb_mutex_allow_trap(VALUE self, int val);
5053
VALUE rb_uninterruptible(VALUE (*b_proc)(VALUE), VALUE data);
5154
VALUE rb_mutex_owned_p(VALUE self);
@@ -64,6 +67,8 @@ void rb_notify_fd_close_wait(struct rb_io_close_wait_list *busy);
6467

6568
RUBY_SYMBOL_EXPORT_BEGIN
6669

70+
void *rb_thread_prevent_fork(void *(*func)(void *), void *data); /* for ext/socket/raddrinfo.c */
71+
6772
/* Temporary. This API will be removed (renamed). */
6873
VALUE rb_thread_io_blocking_region(rb_blocking_function_t *func, void *data1, int fd);
6974
VALUE rb_thread_io_blocking_call(rb_blocking_function_t *func, void *data1, int fd, int events);

process.c

+5
Original file line numberDiff line numberDiff line change
@@ -4227,12 +4227,17 @@ rb_fork_ruby(int *status)
42274227
prefork();
42284228

42294229
before_fork_ruby();
4230+
rb_thread_acquire_fork_lock();
42304231
disable_child_handler_before_fork(&old);
42314232

42324233
child.pid = pid = rb_fork();
42334234
child.error = err = errno;
42344235

42354236
disable_child_handler_fork_parent(&old); /* yes, bad name */
4237+
rb_thread_release_fork_lock();
4238+
if (pid == 0) {
4239+
rb_thread_reset_fork_lock();
4240+
}
42364241
after_fork_ruby(pid);
42374242

42384243
/* repeat while fork failed but retryable */

thread_none.c

+6
Original file line numberDiff line numberDiff line change
@@ -326,4 +326,10 @@ rb_thread_lock_native_thread(void)
326326
return false;
327327
}
328328

329+
void *
330+
rb_thread_prevent_fork(void *(*func)(void *), void *data)
331+
{
332+
return func(data);
333+
}
334+
329335
#endif /* THREAD_SYSTEM_DEPENDENT_IMPLEMENTATION */

thread_pthread.c

+46
Original file line numberDiff line numberDiff line change
@@ -3346,6 +3346,52 @@ native_sleep(rb_thread_t *th, rb_hrtime_t *rel)
33463346
RUBY_DEBUG_LOG("wakeup");
33473347
}
33483348

3349+
// fork read-write lock (only for pthread)
3350+
static pthread_rwlock_t rb_thread_fork_rw_lock = PTHREAD_RWLOCK_INITIALIZER;
3351+
3352+
void
3353+
rb_thread_release_fork_lock(void)
3354+
{
3355+
int r;
3356+
if ((r = pthread_rwlock_unlock(&rb_thread_fork_rw_lock))) {
3357+
rb_bug_errno("pthread_rwlock_unlock", r);
3358+
}
3359+
}
3360+
3361+
void
3362+
rb_thread_reset_fork_lock(void)
3363+
{
3364+
int r;
3365+
if ((r = pthread_rwlock_destroy(&rb_thread_fork_rw_lock))) {
3366+
rb_bug_errno("pthread_rwlock_destroy", r);
3367+
}
3368+
3369+
if ((r = pthread_rwlock_init(&rb_thread_fork_rw_lock, NULL))) {
3370+
rb_bug_errno("pthread_rwlock_init", r);
3371+
}
3372+
}
3373+
3374+
void *
3375+
rb_thread_prevent_fork(void *(*func)(void *), void *data)
3376+
{
3377+
int r;
3378+
if ((r = pthread_rwlock_rdlock(&rb_thread_fork_rw_lock))) {
3379+
rb_bug_errno("pthread_rwlock_rdlock", r);
3380+
}
3381+
void *result = func(data);
3382+
rb_thread_release_fork_lock();
3383+
return result;
3384+
}
3385+
3386+
void
3387+
rb_thread_acquire_fork_lock(void)
3388+
{
3389+
int r;
3390+
if ((r = pthread_rwlock_wrlock(&rb_thread_fork_rw_lock))) {
3391+
rb_bug_errno("pthread_rwlock_wrlock", r);
3392+
}
3393+
}
3394+
33493395
// thread internal event hooks (only for pthread)
33503396

33513397
struct rb_internal_thread_event_hook {

thread_win32.c

+6
Original file line numberDiff line numberDiff line change
@@ -1011,4 +1011,10 @@ rb_thread_lock_native_thread(void)
10111011
return false;
10121012
}
10131013

1014+
void *
1015+
rb_thread_prevent_fork(void *(*func)(void *), void *data)
1016+
{
1017+
return func(data);
1018+
}
1019+
10141020
#endif /* THREAD_SYSTEM_DEPENDENT_IMPLEMENTATION */

0 commit comments

Comments
 (0)