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.
This commit is contained in:
parent
2e5680d304
commit
63cbe3f6ac
Notes:
git
2024-09-05 09:44:04 +00:00
@ -327,6 +327,12 @@ nogvl_getaddrinfo(void *arg)
|
|||||||
return (void *)(VALUE)ret;
|
return (void *)(VALUE)ret;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
static void *
|
||||||
|
fork_safe_getaddrinfo(void *arg)
|
||||||
|
{
|
||||||
|
return rb_thread_prevent_fork(nogvl_getaddrinfo, arg);
|
||||||
|
}
|
||||||
|
|
||||||
static int
|
static int
|
||||||
rb_getaddrinfo(const char *hostp, const char *portp, const struct addrinfo *hints, struct addrinfo **ai)
|
rb_getaddrinfo(const char *hostp, const char *portp, const struct addrinfo *hints, struct addrinfo **ai)
|
||||||
{
|
{
|
||||||
@ -336,7 +342,7 @@ rb_getaddrinfo(const char *hostp, const char *portp, const struct addrinfo *hint
|
|||||||
arg.service = portp;
|
arg.service = portp;
|
||||||
arg.hints = hints;
|
arg.hints = hints;
|
||||||
arg.res = ai;
|
arg.res = ai;
|
||||||
return (int)(VALUE)rb_thread_call_without_gvl(nogvl_getaddrinfo, &arg, RUBY_UBF_IO, 0);
|
return (int)(VALUE)rb_thread_call_without_gvl(fork_safe_getaddrinfo, &arg, RUBY_UBF_IO, 0);
|
||||||
}
|
}
|
||||||
|
|
||||||
#elif GETADDRINFO_IMPL == 2
|
#elif GETADDRINFO_IMPL == 2
|
||||||
@ -477,6 +483,12 @@ do_pthread_create(pthread_t *th, void *(*start_routine) (void *), void *arg)
|
|||||||
return ret;
|
return ret;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
static void *
|
||||||
|
fork_safe_do_getaddrinfo(void *ptr)
|
||||||
|
{
|
||||||
|
return rb_thread_prevent_fork(do_getaddrinfo, ptr);
|
||||||
|
}
|
||||||
|
|
||||||
static int
|
static int
|
||||||
rb_getaddrinfo(const char *hostp, const char *portp, const struct addrinfo *hints, struct addrinfo **ai)
|
rb_getaddrinfo(const char *hostp, const char *portp, const struct addrinfo *hints, struct addrinfo **ai)
|
||||||
{
|
{
|
||||||
@ -493,7 +505,7 @@ start:
|
|||||||
}
|
}
|
||||||
|
|
||||||
pthread_t th;
|
pthread_t th;
|
||||||
if (do_pthread_create(&th, do_getaddrinfo, arg) != 0) {
|
if (do_pthread_create(&th, fork_safe_do_getaddrinfo, arg) != 0) {
|
||||||
int err = errno;
|
int err = errno;
|
||||||
free_getaddrinfo_arg(arg);
|
free_getaddrinfo_arg(arg);
|
||||||
errno = err;
|
errno = err;
|
||||||
|
@ -46,6 +46,9 @@ VALUE rb_thread_shield_wait(VALUE self);
|
|||||||
VALUE rb_thread_shield_release(VALUE self);
|
VALUE rb_thread_shield_release(VALUE self);
|
||||||
VALUE rb_thread_shield_destroy(VALUE self);
|
VALUE rb_thread_shield_destroy(VALUE self);
|
||||||
int rb_thread_to_be_killed(VALUE thread);
|
int rb_thread_to_be_killed(VALUE thread);
|
||||||
|
void rb_thread_acquire_fork_lock(void);
|
||||||
|
void rb_thread_release_fork_lock(void);
|
||||||
|
void rb_thread_reset_fork_lock(void);
|
||||||
void rb_mutex_allow_trap(VALUE self, int val);
|
void rb_mutex_allow_trap(VALUE self, int val);
|
||||||
VALUE rb_uninterruptible(VALUE (*b_proc)(VALUE), VALUE data);
|
VALUE rb_uninterruptible(VALUE (*b_proc)(VALUE), VALUE data);
|
||||||
VALUE rb_mutex_owned_p(VALUE self);
|
VALUE rb_mutex_owned_p(VALUE self);
|
||||||
@ -64,6 +67,8 @@ void rb_notify_fd_close_wait(struct rb_io_close_wait_list *busy);
|
|||||||
|
|
||||||
RUBY_SYMBOL_EXPORT_BEGIN
|
RUBY_SYMBOL_EXPORT_BEGIN
|
||||||
|
|
||||||
|
void *rb_thread_prevent_fork(void *(*func)(void *), void *data); /* for ext/socket/raddrinfo.c */
|
||||||
|
|
||||||
/* Temporary. This API will be removed (renamed). */
|
/* Temporary. This API will be removed (renamed). */
|
||||||
VALUE rb_thread_io_blocking_region(rb_blocking_function_t *func, void *data1, int fd);
|
VALUE rb_thread_io_blocking_region(rb_blocking_function_t *func, void *data1, int fd);
|
||||||
VALUE rb_thread_io_blocking_call(rb_blocking_function_t *func, void *data1, int fd, int events);
|
VALUE rb_thread_io_blocking_call(rb_blocking_function_t *func, void *data1, int fd, int events);
|
||||||
|
@ -4227,12 +4227,17 @@ rb_fork_ruby(int *status)
|
|||||||
prefork();
|
prefork();
|
||||||
|
|
||||||
before_fork_ruby();
|
before_fork_ruby();
|
||||||
|
rb_thread_acquire_fork_lock();
|
||||||
disable_child_handler_before_fork(&old);
|
disable_child_handler_before_fork(&old);
|
||||||
|
|
||||||
child.pid = pid = rb_fork();
|
child.pid = pid = rb_fork();
|
||||||
child.error = err = errno;
|
child.error = err = errno;
|
||||||
|
|
||||||
disable_child_handler_fork_parent(&old); /* yes, bad name */
|
disable_child_handler_fork_parent(&old); /* yes, bad name */
|
||||||
|
rb_thread_release_fork_lock();
|
||||||
|
if (pid == 0) {
|
||||||
|
rb_thread_reset_fork_lock();
|
||||||
|
}
|
||||||
after_fork_ruby(pid);
|
after_fork_ruby(pid);
|
||||||
|
|
||||||
/* repeat while fork failed but retryable */
|
/* repeat while fork failed but retryable */
|
||||||
|
@ -326,4 +326,10 @@ rb_thread_lock_native_thread(void)
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void *
|
||||||
|
rb_thread_prevent_fork(void *(*func)(void *), void *data)
|
||||||
|
{
|
||||||
|
return func(data);
|
||||||
|
}
|
||||||
|
|
||||||
#endif /* THREAD_SYSTEM_DEPENDENT_IMPLEMENTATION */
|
#endif /* THREAD_SYSTEM_DEPENDENT_IMPLEMENTATION */
|
||||||
|
@ -3346,6 +3346,52 @@ native_sleep(rb_thread_t *th, rb_hrtime_t *rel)
|
|||||||
RUBY_DEBUG_LOG("wakeup");
|
RUBY_DEBUG_LOG("wakeup");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// fork read-write lock (only for pthread)
|
||||||
|
static pthread_rwlock_t rb_thread_fork_rw_lock = PTHREAD_RWLOCK_INITIALIZER;
|
||||||
|
|
||||||
|
void
|
||||||
|
rb_thread_release_fork_lock(void)
|
||||||
|
{
|
||||||
|
int r;
|
||||||
|
if ((r = pthread_rwlock_unlock(&rb_thread_fork_rw_lock))) {
|
||||||
|
rb_bug_errno("pthread_rwlock_unlock", r);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
void
|
||||||
|
rb_thread_reset_fork_lock(void)
|
||||||
|
{
|
||||||
|
int r;
|
||||||
|
if ((r = pthread_rwlock_destroy(&rb_thread_fork_rw_lock))) {
|
||||||
|
rb_bug_errno("pthread_rwlock_destroy", r);
|
||||||
|
}
|
||||||
|
|
||||||
|
if ((r = pthread_rwlock_init(&rb_thread_fork_rw_lock, NULL))) {
|
||||||
|
rb_bug_errno("pthread_rwlock_init", r);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
void *
|
||||||
|
rb_thread_prevent_fork(void *(*func)(void *), void *data)
|
||||||
|
{
|
||||||
|
int r;
|
||||||
|
if ((r = pthread_rwlock_rdlock(&rb_thread_fork_rw_lock))) {
|
||||||
|
rb_bug_errno("pthread_rwlock_rdlock", r);
|
||||||
|
}
|
||||||
|
void *result = func(data);
|
||||||
|
rb_thread_release_fork_lock();
|
||||||
|
return result;
|
||||||
|
}
|
||||||
|
|
||||||
|
void
|
||||||
|
rb_thread_acquire_fork_lock(void)
|
||||||
|
{
|
||||||
|
int r;
|
||||||
|
if ((r = pthread_rwlock_wrlock(&rb_thread_fork_rw_lock))) {
|
||||||
|
rb_bug_errno("pthread_rwlock_wrlock", r);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// thread internal event hooks (only for pthread)
|
// thread internal event hooks (only for pthread)
|
||||||
|
|
||||||
struct rb_internal_thread_event_hook {
|
struct rb_internal_thread_event_hook {
|
||||||
|
@ -1011,4 +1011,10 @@ rb_thread_lock_native_thread(void)
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void *
|
||||||
|
rb_thread_prevent_fork(void *(*func)(void *), void *data)
|
||||||
|
{
|
||||||
|
return func(data);
|
||||||
|
}
|
||||||
|
|
||||||
#endif /* THREAD_SYSTEM_DEPENDENT_IMPLEMENTATION */
|
#endif /* THREAD_SYSTEM_DEPENDENT_IMPLEMENTATION */
|
||||||
|
Loading…
x
Reference in New Issue
Block a user