Revert SIGCHLD changes to diagnose CI failures. (#7517)

* Revert "Remove special handling of `SIGCHLD`. (#7482)"

This reverts commit 44a0711eab7fbc71ac2c8ff489d8c53e97a8fe75.

* Revert "Remove prototypes for functions that are no longer used. (#7497)"

This reverts commit 4dce12bead3bfd91fd80b5e7195f7f540ffffacb.

* Revert "Remove SIGCHLD `waidpid`. (#7476)"

This reverts commit 1658e7d96696a656d9bd0a0c84c82cde86914ba2.

* Fix change to rjit variable name.
This commit is contained in:
Samuel Williams 2023-03-14 20:07:59 +13:00 committed by GitHub
parent b27793835b
commit ac65ce16e9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
Notes: git 2023-03-14 07:08:29 +00:00
Merged-By: ioquatix <samuel@codeotaku.com>
8 changed files with 381 additions and 8 deletions

213
process.c
View File

@ -1061,6 +1061,8 @@ do_waitpid(rb_pid_t pid, int *st, int flags)
#endif
}
#define WAITPID_LOCK_ONLY ((struct waitpid_state *)-1)
struct waitpid_state {
struct ccan_list_node wnode;
rb_execution_context_t *ec;
@ -1072,6 +1074,103 @@ struct waitpid_state {
int errnum;
};
int rb_sigwait_fd_get(const rb_thread_t *);
void rb_sigwait_sleep(const rb_thread_t *, int fd, const rb_hrtime_t *);
void rb_sigwait_fd_put(const rb_thread_t *, int fd);
void rb_thread_sleep_interruptible(void);
static int
waitpid_signal(struct waitpid_state *w)
{
if (w->ec) { /* rb_waitpid */
rb_threadptr_interrupt(rb_ec_thread_ptr(w->ec));
return TRUE;
}
return FALSE;
}
// Used for VM memsize reporting. Returns the size of a list of waitpid_state
// structs. Defined here because the struct definition lives here as well.
size_t
rb_vm_memsize_waiting_list(struct ccan_list_head *waiting_list)
{
struct waitpid_state *waitpid = 0;
size_t size = 0;
ccan_list_for_each(waiting_list, waitpid, wnode) {
size += sizeof(struct waitpid_state);
}
return size;
}
/*
* When a thread is done using sigwait_fd and there are other threads
* sleeping on waitpid, we must kick one of the threads out of
* rb_native_cond_wait so it can switch to rb_sigwait_sleep
*/
static void
sigwait_fd_migrate_sleeper(rb_vm_t *vm)
{
struct waitpid_state *w = 0;
ccan_list_for_each(&vm->waiting_pids, w, wnode) {
if (waitpid_signal(w)) return;
}
ccan_list_for_each(&vm->waiting_grps, w, wnode) {
if (waitpid_signal(w)) return;
}
}
void
rb_sigwait_fd_migrate(rb_vm_t *vm)
{
rb_native_mutex_lock(&vm->waitpid_lock);
sigwait_fd_migrate_sleeper(vm);
rb_native_mutex_unlock(&vm->waitpid_lock);
}
#if RUBY_SIGCHLD
extern volatile unsigned int ruby_nocldwait; /* signal.c */
/* called by timer thread or thread which acquired sigwait_fd */
static void
waitpid_each(struct ccan_list_head *head)
{
struct waitpid_state *w = 0, *next;
ccan_list_for_each_safe(head, w, next, wnode) {
rb_pid_t ret = do_waitpid(w->pid, &w->status, w->options | WNOHANG);
if (!ret) continue;
if (ret == -1) w->errnum = errno;
w->ret = ret;
ccan_list_del_init(&w->wnode);
waitpid_signal(w);
}
}
#else
# define ruby_nocldwait 0
#endif
void
ruby_waitpid_all(rb_vm_t *vm)
{
#if RUBY_SIGCHLD
rb_native_mutex_lock(&vm->waitpid_lock);
waitpid_each(&vm->waiting_pids);
if (ccan_list_empty(&vm->waiting_pids)) {
waitpid_each(&vm->waiting_grps);
}
/* emulate SA_NOCLDWAIT */
if (ccan_list_empty(&vm->waiting_pids) && ccan_list_empty(&vm->waiting_grps)) {
while (ruby_nocldwait && do_waitpid(-1, 0, WNOHANG) > 0)
; /* keep looping */
}
rb_native_mutex_unlock(&vm->waitpid_lock);
#endif
}
static void
waitpid_state_init(struct waitpid_state *w, rb_pid_t pid, int options)
{
@ -1082,6 +1181,77 @@ waitpid_state_init(struct waitpid_state *w, rb_pid_t pid, int options)
w->status = 0;
}
static VALUE
waitpid_sleep(VALUE x)
{
struct waitpid_state *w = (struct waitpid_state *)x;
while (!w->ret) {
rb_thread_sleep_interruptible();
}
return Qfalse;
}
static VALUE
waitpid_cleanup(VALUE x)
{
struct waitpid_state *w = (struct waitpid_state *)x;
/*
* XXX w->ret is sometimes set but ccan_list_del is still needed, here,
* Not sure why, so we unconditionally do ccan_list_del here:
*/
if (TRUE || w->ret == 0) {
rb_vm_t *vm = rb_ec_vm_ptr(w->ec);
rb_native_mutex_lock(&vm->waitpid_lock);
ccan_list_del(&w->wnode);
rb_native_mutex_unlock(&vm->waitpid_lock);
}
return Qfalse;
}
static void
waitpid_wait(struct waitpid_state *w)
{
rb_vm_t *vm = rb_ec_vm_ptr(w->ec);
int need_sleep = FALSE;
/*
* Lock here to prevent do_waitpid from stealing work from the
* ruby_waitpid_locked done by rjit workers since rjit works
* outside of GVL
*/
rb_native_mutex_lock(&vm->waitpid_lock);
if (w->pid > 0 || ccan_list_empty(&vm->waiting_pids)) {
w->ret = do_waitpid(w->pid, &w->status, w->options | WNOHANG);
}
if (w->ret) {
if (w->ret == -1) w->errnum = errno;
}
else if (w->options & WNOHANG) {
}
else {
need_sleep = TRUE;
}
if (need_sleep) {
w->cond = 0;
/* order matters, favor specified PIDs rather than -1 or 0 */
ccan_list_add(w->pid > 0 ? &vm->waiting_pids : &vm->waiting_grps, &w->wnode);
}
rb_native_mutex_unlock(&vm->waitpid_lock);
if (need_sleep) {
rb_ensure(waitpid_sleep, (VALUE)w, waitpid_cleanup, (VALUE)w);
}
}
static void *
waitpid_blocking_no_SIGCHLD(void *x)
{
@ -1100,7 +1270,8 @@ waitpid_no_SIGCHLD(struct waitpid_state *w)
}
else {
do {
rb_thread_call_without_gvl(waitpid_blocking_no_SIGCHLD, w, RUBY_UBF_PROCESS, 0);
rb_thread_call_without_gvl(waitpid_blocking_no_SIGCHLD, w,
RUBY_UBF_PROCESS, 0);
} while (w->ret < 0 && errno == EINTR && (RUBY_VM_CHECK_INTS(w->ec),1));
}
if (w->ret == -1)
@ -1122,10 +1293,20 @@ rb_process_status_wait(rb_pid_t pid, int flags)
waitpid_state_init(&waitpid_state, pid, flags);
waitpid_state.ec = GET_EC();
if (WAITPID_USE_SIGCHLD) {
waitpid_wait(&waitpid_state);
}
else {
waitpid_no_SIGCHLD(&waitpid_state);
}
if (waitpid_state.ret == 0) return Qnil;
if (waitpid_state.ret > 0 && ruby_nocldwait) {
waitpid_state.ret = -1;
waitpid_state.errnum = ECHILD;
}
return rb_process_status_new(waitpid_state.ret, waitpid_state.status, waitpid_state.errnum);
}
@ -3925,10 +4106,16 @@ retry_fork_async_signal_safe(struct rb_process_status *status, int *ep,
volatile int try_gc = 1;
struct child_handler_disabler_state old;
int err;
rb_nativethread_lock_t *const volatile waitpid_lock_init =
(w && WAITPID_USE_SIGCHLD) ? &GET_VM()->waitpid_lock : 0;
while (1) {
rb_nativethread_lock_t *waitpid_lock = waitpid_lock_init;
prefork();
disable_child_handler_before_fork(&old);
if (waitpid_lock) {
rb_native_mutex_lock(waitpid_lock);
}
#ifdef HAVE_WORKING_VFORK
if (!has_privilege())
pid = vfork();
@ -3953,6 +4140,14 @@ retry_fork_async_signal_safe(struct rb_process_status *status, int *ep,
#endif
}
err = errno;
waitpid_lock = waitpid_lock_init;
if (waitpid_lock) {
if (pid > 0 && w != WAITPID_LOCK_ONLY) {
w->pid = pid;
ccan_list_add(&GET_VM()->waiting_pids, &w->wnode);
}
rb_native_mutex_unlock(waitpid_lock);
}
disable_child_handler_fork_parent(&old);
if (0 < pid) /* fork succeed, parent process */
return pid;
@ -3997,12 +4192,13 @@ fork_check_err(struct rb_process_status *status, int (*chfunc)(void*, char *, si
int state = 0;
status->error = err;
VM_ASSERT((w == 0) && "only used by extensions");
VM_ASSERT((w == 0 || w == WAITPID_LOCK_ONLY) &&
"only used by extensions");
rb_protect(proc_syswait, (VALUE)pid, &state);
status->status = state;
}
else if (!w) {
else if (!w || w == WAITPID_LOCK_ONLY) {
rb_syswait(pid);
}
@ -4432,7 +4628,7 @@ rb_spawn_process(struct rb_execarg *eargp, char *errmsg, size_t errmsg_buflen)
rb_last_status_set((status & 0xff) << 8, pid);
# endif
if (eargp->waitpid_state) {
if (eargp->waitpid_state && eargp->waitpid_state != WAITPID_LOCK_ONLY) {
eargp->waitpid_state->pid = pid;
}
@ -4463,6 +4659,15 @@ static rb_pid_t
rb_execarg_spawn(VALUE execarg_obj, char *errmsg, size_t errmsg_buflen)
{
struct spawn_args args;
struct rb_execarg *eargp = rb_execarg_get(execarg_obj);
/*
* Prevent a race with RJIT where the compiler process where
* can hold an FD of ours in between vfork + execve
*/
if (!eargp->waitpid_state && rb_rjit_enabled) {
eargp->waitpid_state = WAITPID_LOCK_ONLY;
}
args.execarg = execarg_obj;
args.errmsg.ptr = errmsg;

View File

@ -508,6 +508,9 @@ static struct {
rb_atomic_t cnt[RUBY_NSIG];
rb_atomic_t size;
} signal_buff;
#if RUBY_SIGCHLD
volatile unsigned int ruby_nocldwait;
#endif
#define sighandler_t ruby_sighandler_t
@ -605,6 +608,27 @@ ruby_signal(int signum, sighandler_t handler)
#endif
switch (signum) {
#if RUBY_SIGCHLD
case RUBY_SIGCHLD:
if (handler == SIG_IGN) {
ruby_nocldwait = 1;
# ifdef USE_SIGALTSTACK
if (sigact.sa_flags & SA_SIGINFO) {
sigact.sa_sigaction = (ruby_sigaction_t*)sighandler;
}
else {
sigact.sa_handler = sighandler;
}
# else
sigact.sa_handler = handler;
sigact.sa_flags = 0;
# endif
}
else {
ruby_nocldwait = 0;
}
break;
#endif
#if defined(SA_ONSTACK) && defined(USE_SIGALTSTACK)
case SIGSEGV:
#ifdef SIGBUS
@ -683,14 +707,35 @@ signal_enque(int sig)
ATOMIC_INC(signal_buff.size);
}
#if RUBY_SIGCHLD
static rb_atomic_t sigchld_hit;
/* destructive getter than simple predicate */
# define GET_SIGCHLD_HIT() ATOMIC_EXCHANGE(sigchld_hit, 0)
#else
# define GET_SIGCHLD_HIT() 0
#endif
static void
sighandler(int sig)
{
int old_errnum = errno;
signal_enque(sig);
rb_thread_wakeup_timer_thread(sig);
/* the VM always needs to handle SIGCHLD for rb_waitpid */
if (sig == RUBY_SIGCHLD) {
#if RUBY_SIGCHLD
rb_vm_t *vm = GET_VM();
ATOMIC_EXCHANGE(sigchld_hit, 1);
/* avoid spurious wakeup in main thread if and only if nobody uses trap(:CHLD) */
if (vm && ACCESS_ONCE(VALUE, vm->trap_list.cmd[sig])) {
signal_enque(sig);
}
#endif
}
else {
signal_enque(sig);
}
rb_thread_wakeup_timer_thread(sig);
#if !defined(BSD_SIGNAL) && !defined(POSIX_SIGNAL)
ruby_signal(sig, sighandler);
#endif
@ -1040,6 +1085,16 @@ rb_vm_trap_exit(rb_vm_t *vm)
}
}
void ruby_waitpid_all(rb_vm_t *); /* process.c */
void
ruby_sigchld_handler(rb_vm_t *vm)
{
if (SIGCHLD_LOSSY || GET_SIGCHLD_HIT()) {
ruby_waitpid_all(vm);
}
}
/* returns true if a trap handler was run, false otherwise */
int
rb_signal_exec(rb_thread_t *th, int sig)
@ -1106,6 +1161,9 @@ default_handler(int sig)
#endif
#ifdef SIGUSR2
case SIGUSR2:
#endif
#if RUBY_SIGCHLD
case RUBY_SIGCHLD:
#endif
func = sighandler;
break;
@ -1172,6 +1230,9 @@ trap_handler(VALUE *cmd, int sig)
break;
case 14:
if (memcmp(cptr, "SYSTEM_DEFAULT", 14) == 0) {
if (sig == RUBY_SIGCHLD) {
goto sig_dfl;
}
func = SIG_DFL;
*cmd = 0;
}
@ -1562,5 +1623,33 @@ fake_grantfd(int masterfd)
int
rb_grantpt(int masterfd)
{
if (RUBY_SIGCHLD) {
rb_vm_t *vm = GET_VM();
int ret, e;
/*
* Prevent waitpid calls from Ruby by taking waitpid_lock.
* Pedantically, grantpt(3) is undefined if a non-default
* SIGCHLD handler is defined, but preventing conflicting
* waitpid calls ought to be sufficient.
*
* We could install the default sighandler temporarily, but that
* could cause SIGCHLD to be missed by other threads. Blocking
* SIGCHLD won't work here, either, unless we stop and restart
* timer-thread (as only timer-thread sees SIGCHLD), but that
* seems like overkill.
*/
rb_nativethread_lock_lock(&vm->waitpid_lock);
{
ret = grantpt(masterfd); /* may spawn `pt_chown' and wait on it */
if (ret < 0) e = errno;
}
rb_nativethread_lock_unlock(&vm->waitpid_lock);
if (ret < 0) errno = e;
return ret;
}
else {
return grantpt(masterfd);
}
}

View File

@ -323,6 +323,49 @@ class TestSignal < Test::Unit::TestCase
end;
end
def test_sigchld_ignore
omit 'no SIGCHLD' unless Signal.list['CHLD']
old = trap(:CHLD, 'IGNORE')
cmd = [ EnvUtil.rubybin, '--disable=gems', '-e' ]
assert(system(*cmd, 'exit!(0)'), 'no ECHILD')
IO.pipe do |r, w|
pid = spawn(*cmd, "STDIN.read", in: r)
nb = Process.wait(pid, Process::WNOHANG)
th = Thread.new(Thread.current) do |parent|
Thread.pass until parent.stop? # wait for parent to Process.wait
w.close
end
assert_raise(Errno::ECHILD) { Process.wait(pid) }
th.join
assert_nil nb
end
IO.pipe do |r, w|
pids = 3.times.map { spawn(*cmd, 'exit!', out: w) }
w.close
zombies = pids.dup
assert_nil r.read(1), 'children dead'
Timeout.timeout(10) do
zombies.delete_if do |pid|
begin
Process.kill(0, pid)
false
rescue Errno::ESRCH
true
end
end while zombies[0]
end
assert_predicate zombies, :empty?, 'zombies leftover'
pids.each do |pid|
assert_raise(Errno::ECHILD) { Process.waitpid(pid) }
end
end
ensure
trap(:CHLD, old) if Signal.list['CHLD']
end
def test_sigwait_fd_unused
t = EnvUtil.apply_timeout_scale(0.1)
assert_separately([], <<-End)

View File

@ -145,6 +145,7 @@ static int hrtime_update_expire(rb_hrtime_t *, const rb_hrtime_t);
NORETURN(static void async_bug_fd(const char *mesg, int errno_arg, int fd));
static int consume_communication_pipe(int fd);
static int check_signals_nogvl(rb_thread_t *, int sigwait_fd);
void rb_sigwait_fd_migrate(rb_vm_t *); /* process.c */
#define eKillSignal INT2FIX(0)
#define eTerminateSignal INT2FIX(1)
@ -257,6 +258,7 @@ timeout_prepare(rb_hrtime_t **to, rb_hrtime_t *rel, rb_hrtime_t *end,
}
MAYBE_UNUSED(NOINLINE(static int thread_start_func_2(rb_thread_t *th, VALUE *stack_start)));
void ruby_sigchld_handler(rb_vm_t *); /* signal.c */
static void
ubf_sigwait(void *ignore)
@ -1367,6 +1369,18 @@ rb_thread_sleep_deadly(void)
sleep_forever(GET_THREAD(), SLEEP_DEADLOCKABLE|SLEEP_SPURIOUS_CHECK);
}
void
rb_thread_sleep_interruptible(void)
{
rb_thread_t *th = GET_THREAD();
enum rb_thread_status prev_status = th->status;
th->status = THREAD_STOPPED;
native_sleep(th, 0);
RUBY_VM_CHECK_INTS_BLOCKING(th->ec);
th->status = prev_status;
}
static void
rb_thread_sleep_deadly_allow_spurious_wakeup(VALUE blocker, VALUE timeout, rb_hrtime_t end)
{
@ -2304,7 +2318,9 @@ rb_threadptr_execute_interrupts(rb_thread_t *th, int blocking_timing)
if (sigwait_fd >= 0) {
(void)consume_communication_pipe(sigwait_fd);
ruby_sigchld_handler(th->vm);
rb_sigwait_fd_put(th, sigwait_fd);
rb_sigwait_fd_migrate(th->vm);
}
th->status = THREAD_RUNNABLE;
while ((sig = rb_get_next_signal()) != 0) {
@ -4054,6 +4070,7 @@ select_set_free(VALUE p)
if (set->sigwait_fd >= 0) {
rb_sigwait_fd_put(set->th, set->sigwait_fd);
rb_sigwait_fd_migrate(set->th->vm);
}
rb_fd_term(&set->orig_rset);
@ -4269,6 +4286,7 @@ rb_thread_wait_for_single_fd(int fd, int events, struct timeval *timeout)
int fd1 = sigwait_signals_fd(result, fds[1].revents, fds[1].fd);
(void)check_signals_nogvl(wfd.th, fd1);
rb_sigwait_fd_put(wfd.th, fds[1].fd);
rb_sigwait_fd_migrate(wfd.th->vm);
}
RUBY_VM_CHECK_INTS_BLOCKING(wfd.th->ec);
} while (wait_retryable(&result, lerrno, to, end));
@ -4488,6 +4506,7 @@ check_signals_nogvl(rb_thread_t *th, int sigwait_fd)
rb_vm_t *vm = GET_VM(); /* th may be 0 */
int ret = sigwait_fd >= 0 ? consume_communication_pipe(sigwait_fd) : FALSE;
ubf_wakeup_all_threads();
ruby_sigchld_handler(vm);
if (rb_signal_buff_size()) {
if (th == vm->ractor.main_thread) {
/* no need to lock + wakeup if already in main thread */
@ -4588,6 +4607,7 @@ rb_thread_atfork_internal(rb_thread_t *th, void (*atfork)(rb_thread_t *, const r
rb_ractor_atfork(vm, th);
/* may be held by RJIT threads in parent */
rb_native_mutex_initialize(&vm->waitpid_lock);
rb_native_mutex_initialize(&vm->workqueue_lock);
/* may be held by any thread in parent */
@ -5254,6 +5274,7 @@ Init_Thread_Mutex(void)
{
rb_thread_t *th = GET_THREAD();
rb_native_mutex_initialize(&th->vm->waitpid_lock);
rb_native_mutex_initialize(&th->vm->workqueue_lock);
rb_native_mutex_initialize(&th->interrupt_lock);
}

View File

@ -348,6 +348,7 @@ do_gvl_timer(struct rb_thread_sched *sched, rb_thread_t *th)
sched->timer_err = native_cond_timedwait(&th->nt->cond.readyq, &sched->lock, &abs);
ubf_wakeup_all_threads();
ruby_sigchld_handler(vm);
if (UNLIKELY(rb_signal_buff_size())) {
if (th == vm->ractor.main_thread) {
@ -2358,6 +2359,7 @@ native_sleep(rb_thread_t *th, rb_hrtime_t *rel)
THREAD_BLOCKING_END(th);
rb_sigwait_fd_put(th, sigwait_fd);
rb_sigwait_fd_migrate(th->vm);
}
else if (th == th->vm->ractor.main_thread) { /* always able to handle signals */
native_ppoll_sleep(th, rel);

View File

@ -753,6 +753,7 @@ timer_thread_func(void *dummy)
while (WaitForSingleObject(timer_thread.lock,
TIME_QUANTUM_USEC/1000) == WAIT_TIMEOUT) {
vm->clock++;
ruby_sigchld_handler(vm); /* probably no-op */
rb_threadptr_check_signal(vm->ractor.main_thread);
}
RUBY_DEBUG_LOG("end");

4
vm.c
View File

@ -2885,6 +2885,7 @@ ruby_vm_destruct(rb_vm_t *vm)
if (objspace) {
rb_objspace_free(objspace);
}
rb_native_mutex_destroy(&vm->waitpid_lock);
rb_native_mutex_destroy(&vm->workqueue_lock);
/* after freeing objspace, you *can't* use ruby_xfree() */
ruby_mimfree(vm);
@ -2894,6 +2895,7 @@ ruby_vm_destruct(rb_vm_t *vm)
return 0;
}
size_t rb_vm_memsize_waiting_list(struct ccan_list_head *waiting_list); // process.c
size_t rb_vm_memsize_waiting_fds(struct ccan_list_head *waiting_fds); // thread.c
size_t rb_vm_memsize_postponed_job_buffer(void); // vm_trace.c
size_t rb_vm_memsize_workqueue(struct ccan_list_head *workqueue); // vm_trace.c
@ -2950,6 +2952,8 @@ vm_memsize(const void *ptr)
return (
sizeof(rb_vm_t) +
rb_vm_memsize_waiting_list(&vm->waiting_pids) +
rb_vm_memsize_waiting_list(&vm->waiting_grps) +
rb_vm_memsize_waiting_fds(&vm->waiting_fds) +
rb_st_memsize(vm->loaded_features_index) +
rb_st_memsize(vm->loading_table) +

View File

@ -145,6 +145,9 @@ extern int ruby_assert_critical_section_entered;
# define SIGCHLD_LOSSY (0)
#endif
/* define to 0 to test old code path */
#define WAITPID_USE_SIGCHLD (RUBY_SIGCHLD || SIGCHLD_LOSSY)
#if defined(SIGSEGV) && defined(HAVE_SIGALTSTACK) && defined(SA_SIGINFO) && !defined(__NetBSD__)
# define USE_SIGALTSTACK
void *rb_allocate_sigaltstack(void);
@ -649,6 +652,9 @@ typedef struct rb_vm_struct {
#endif
rb_serial_t fork_gen;
rb_nativethread_lock_t waitpid_lock;
struct ccan_list_head waiting_pids; /* PID > 0: <=> struct waitpid_state */
struct ccan_list_head waiting_grps; /* PID <= 0: <=> struct waitpid_state */
struct ccan_list_head waiting_fds; /* <=> struct waiting_fd */
/* set in single-threaded processes only: */
@ -1756,7 +1762,9 @@ static inline void
rb_vm_living_threads_init(rb_vm_t *vm)
{
ccan_list_head_init(&vm->waiting_fds);
ccan_list_head_init(&vm->waiting_pids);
ccan_list_head_init(&vm->workqueue);
ccan_list_head_init(&vm->waiting_grps);
ccan_list_head_init(&vm->ractor.set);
}