summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authornormal <normal@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>2018-11-01 14:10:47 +0000
committernormal <normal@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>2018-11-01 14:10:47 +0000
commit5de7b3b4f27df747899c243adbb10c9799ad1399 (patch)
tree23639a8507b406bf9bc8b80697d7b3aaad568b99
parentf6d44b59c6ecb47047ec15e1541de57e191186e0 (diff)
thread_pthread.c (native_ppoll_sleep): new eventfd (or pipe) for ubf
Relying on ubf_select + ubf_list for main thread is not guaranteed to wake a process up as it does not acquire sigwait_fd and all other threads may be sleeping. native_cond_sleep and the sigwait_fd path are immune to TOCTOU issues, but native_ppoll_sleep may have its wakeup stolen by sigwait_fd sleeper and the RUBY_VM_INTERRUPTED check is insufficient. Note: for pthreads platforms without POSIX timers, this becomes more expensive than Ruby 2.5, as six pipe FDs come into use. Linux is best off with only two descriptors for eventfd. [ruby-core:89655] cf. http://ci.rvm.jp/results/trunk-mjit@silicon-docker/1437559 http://ci.rvm.jp/results/trunk-mjit-wait@silicon-docker/1437673 git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@65495 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
-rw-r--r--thread_pthread.c40
1 files changed, 30 insertions, 10 deletions
diff --git a/thread_pthread.c b/thread_pthread.c
index 97d6eafa31..22ca4c2e3f 100644
--- a/thread_pthread.c
+++ b/thread_pthread.c
@@ -1395,11 +1395,13 @@ static int ubf_threads_empty(void) { return 1; }
static struct {
/* pipes are closed in forked children when owner_process does not match */
int normal[2]; /* [0] == sigwait_fd */
+ int ub_main[2]; /* unblock main thread from native_ppoll_sleep */
/* volatile for signal handler use: */
volatile rb_pid_t owner_process;
} signal_self_pipe = {
{-1, -1},
+ {-1, -1},
};
/* only use signal-safe system calls here */
@@ -1717,10 +1719,12 @@ rb_thread_create_timer_thread(void)
if (owner && owner != current) {
CLOSE_INVALIDATE_PAIR(signal_self_pipe.normal);
+ CLOSE_INVALIDATE_PAIR(signal_self_pipe.ub_main);
ubf_timer_invalidate();
}
if (setup_communication_pipe_internal(signal_self_pipe.normal) < 0) return;
+ if (setup_communication_pipe_internal(signal_self_pipe.ub_main) < 0) return;
if (owner != current) {
/* validate pipe on this process */
@@ -1844,7 +1848,8 @@ rb_reserved_fd_p(int fd)
#endif
if (fd == signal_self_pipe.normal[0] || fd == signal_self_pipe.normal[1])
goto check_pid;
-
+ if (fd == signal_self_pipe.ub_main[0] || fd == signal_self_pipe.ub_main[1])
+ goto check_pid;
return 0;
check_pid:
if (signal_self_pipe.owner_process == getpid()) /* async-signal-safe */
@@ -1993,6 +1998,17 @@ rb_sigwait_sleep(rb_thread_t *th, int sigwait_fd, const rb_hrtime_t *rel)
}
/*
+ * we need to guarantee wakeups from native_ppoll_sleep because
+ * ubf_select may not be going through ubf_list if other threads
+ * are all sleeping.
+ */
+static void
+ubf_ppoll_sleep(void *ignore)
+{
+ rb_thread_wakeup_timer_thread_fd(signal_self_pipe.ub_main[1]);
+}
+
+/*
* This function does not exclusively acquire sigwait_fd, so it
* cannot safely read from it. However, it can be woken up in
* 4 ways:
@@ -2006,22 +2022,26 @@ static void
native_ppoll_sleep(rb_thread_t *th, rb_hrtime_t *rel)
{
rb_native_mutex_lock(&th->interrupt_lock);
- th->unblock.func = ubf_select;
- th->unblock.arg = th;
+ th->unblock.func = ubf_ppoll_sleep;
rb_native_mutex_unlock(&th->interrupt_lock);
GVL_UNLOCK_BEGIN(th);
if (!RUBY_VM_INTERRUPTED(th->ec)) {
- struct pollfd pfd;
+ struct pollfd pfd[2];
struct timespec ts;
- pfd.fd = signal_self_pipe.normal[0]; /* sigwait_fd */
- pfd.events = POLLIN;
- (void)ppoll(&pfd, 1, rb_hrtime2timespec(&ts, rel), 0);
-
+ pfd[0].fd = signal_self_pipe.normal[0]; /* sigwait_fd */
+ pfd[1].fd = signal_self_pipe.ub_main[0];
+ pfd[0].events = pfd[1].events = POLLIN;
+ if (ppoll(pfd, 2, rb_hrtime2timespec(&ts, rel), 0) > 0) {
+ if (pfd[1].revents & POLLIN) {
+ (void)consume_communication_pipe(pfd[1].fd);
+ }
+ }
/*
- * do not read the fd, here, let uplevel callers or other threads
- * that, otherwise we may steal and starve other threads
+ * do not read the sigwait_fd, here, let uplevel callers
+ * or other threads that, otherwise we may steal and starve
+ * other threads
*/
}
unblock_function_clear(th);