Bug #11362: [PATCH] ensure Process.kill(:STOP, $$) is resumable

• Status: Closed
• Assignee: normalperson (Eric Wong)
• Target version: -

[normalperson (Eric Wong)]

make Process.kill(:STOP, $$) resumable

Self-inflicted signals are delivered immediately. This is fine
for most signals which are catchable, but SIGSTOP and SIGKILL
are special and cannot be caught by a userspace process.

SIGKILL is easy, the process will die immediately and we won't
care for it. However, SIGSTOP is tricky because we cannot know
when it is delivered.

Thus, we must rely on sighandler->timer_thread to signal
th->interrupt_cond when SIGCONT resumes the process.

• signal.c (Init_signal): install sighandler for SIGCONT
• test/ruby/test_process.rb (test_stop_self_resumable): new test

Will commit unless there's a better way. I found this bug
while looking into making signal handling work in single-threaded
processes without relying on timer thread.

[normalperson (Eric Wong)]

Alternative patch, but I relying on sched_yield() may be unreliable.
Anyways, both patches are hacky; but so is the expected behavior
of immediate delivery when a process signals itself...

--- a/thread.c
+++ b/thread.c
@@ -5262,6 +5262,11 @@ ruby_kill(rb_pid_t pid, int sig)
 {
     int err;
     rb_thread_t *th = GET_THREAD();
+#ifdef SIGSTOP
+    int sigstop = sig == SIGSTOP;
+#else
+    int sigstop = 0;
+#endif
 
     /*
      * When target pid is self, many caller assume signal will be
@@ -5271,7 +5276,16 @@ ruby_kill(rb_pid_t pid, int sig)
 	GVL_UNLOCK_BEGIN();
 	native_mutex_lock(&th->interrupt_lock);
 	err = kill(pid, sig);
-	native_cond_wait(&th->interrupt_cond, &th->interrupt_lock);
+	if (sigstop) {
+	    /*
+	     * best effort to try to receive SIGSTOP ASAP,
+	     * maybe we need to yield several more times.
+	     */
+	    native_thread_yield();
+	}
+	else { /* sig is SIGKILL, SIGSEGV, or SIGBUS: wait to die */
+	    native_cond_wait(&th->interrupt_cond, &th->interrupt_lock);
+	}
 	native_mutex_unlock(&th->interrupt_lock);
 	GVL_UNLOCK_END();
     }

[normalperson (Eric Wong)]

And I forget we probably need to deal with SYSTEM_DEFAULT, too.
Ugh. I almost favor my alternative patch to use sched_yield
instead of overriding SIGCONT.

My alternative patch would allow removing th->interrupt_cond
entirely and just use select(0, NULL, NULL, NULL, NULL) to sleep
infinitely for the SEGV/KILL/BUS cases.

[normalperson (Eric Wong)]

• Status changed from Open to Closed
• Backport deleted (2.0.0: REQUIRED, 2.1: REQUIRED, 2.2: REQUIRED)