Skip to content

Commit eab4a0b

Browse files
authored
Fix race condition in signal handler query (ruby#13712)
* Fix race condition in signal handler query * Initialize signal lock dynamically and reset after fork * Fix signal handler mutex initialization conditions
1 parent 31c1f36 commit eab4a0b

File tree

3 files changed

+21
-2
lines changed

3 files changed

+21
-2
lines changed

internal/signal.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ void (*ruby_posix_signal(int, void (*)(int)))(int);
1919

2020
RUBY_SYMBOL_EXPORT_BEGIN
2121
/* signal.c (export) */
22+
void rb_signal_atfork(void);
2223
RUBY_SYMBOL_EXPORT_END
2324

2425
#endif /* INTERNAL_SIGNAL_H */

signal.c

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -664,6 +664,10 @@ ruby_nativethread_signal(int signum, sighandler_t handler)
664664
#endif
665665
#endif
666666

667+
#if !defined(POSIX_SIGNAL) && !defined(SIG_GET)
668+
static rb_nativethread_lock_t sig_check_lock;
669+
#endif
670+
667671
static int
668672
signal_ignored(int sig)
669673
{
@@ -678,9 +682,11 @@ signal_ignored(int sig)
678682
// SIG_GET: Returns the current value of the signal.
679683
func = signal(sig, SIG_GET);
680684
#else
681-
// TODO: this is not a thread-safe way to do it. Needs lock.
682-
sighandler_t old = signal(sig, SIG_DFL);
685+
sighandler_t old;
686+
rb_native_mutex_lock(&sig_check_lock);
687+
old = signal(sig, SIG_DFL);
683688
signal(sig, old);
689+
rb_native_mutex_unlock(&sig_check_lock);
684690
func = old;
685691
#endif
686692
if (func == SIG_IGN) return 1;
@@ -1509,6 +1515,9 @@ Init_signal(void)
15091515
rb_define_method(rb_eSignal, "signo", esignal_signo, 0);
15101516
rb_alias(rb_eSignal, rb_intern_const("signm"), rb_intern_const("message"));
15111517
rb_define_method(rb_eInterrupt, "initialize", interrupt_init, -1);
1518+
#if !defined(POSIX_SIGNAL) && !defined(SIG_GET)
1519+
rb_native_mutex_initialize(&sig_check_lock);
1520+
#endif
15121521

15131522
// It should be ready to call rb_signal_exec()
15141523
VM_ASSERT(GET_THREAD()->pending_interrupt_queue);
@@ -1561,3 +1570,11 @@ Init_signal(void)
15611570

15621571
rb_enable_interrupt();
15631572
}
1573+
1574+
void
1575+
rb_signal_atfork(void)
1576+
{
1577+
#if defined(HAVE_WORKING_FORK) && !defined(POSIX_SIGNAL) && !defined(SIG_GET)
1578+
rb_native_mutex_initialize(&sig_check_lock);
1579+
#endif
1580+
}

thread.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4933,6 +4933,7 @@ rb_thread_atfork_internal(rb_thread_t *th, void (*atfork)(rb_thread_t *, const r
49334933

49344934
thread_sched_atfork(TH_SCHED(th));
49354935
ubf_list_atfork();
4936+
rb_signal_atfork();
49364937

49374938
// OK. Only this thread accesses:
49384939
ccan_list_for_each(&vm->ractor.set, r, vmlr_node) {

0 commit comments

Comments
 (0)