Project

General

Profile

Actions

Bug #11362

closed

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

Added by normalperson (Eric Wong) over 8 years ago. Updated about 6 years ago.

Status:
Closed
Target version:
-
ruby -v:
Backport:
[ruby-core:70011]

Description

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.


Files

0001-make-Process.kill-STOP-resumable.patch (2.65 KB) 0001-make-Process.kill-STOP-resumable.patch normalperson (Eric Wong), 07/17/2015 10:05 AM

Updated by normalperson (Eric Wong) over 8 years ago

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();
     }

Updated by normalperson (Eric Wong) over 8 years ago

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.

Actions #3

Updated by normalperson (Eric Wong) about 6 years ago

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

Also available in: Atom PDF

Like0
Like0Like0Like0