From 103257ccca836dc0ea2fece14334bc73cb034e6f Mon Sep 17 00:00:00 2001 From: Theodore Dubois Date: Sat, 18 Apr 2020 14:44:39 -0700 Subject: [PATCH] Make sure all callers of wait_for check the return value If you don't and you get a signal and ignore the _EINTR, you'll just spin at 100% CPU. SSH with a control master (used by git annex) appears to hit this with the unix_got_peer wait. --- fs/sock.c | 2 +- kernel/eventfd.c | 10 ++++++++-- util/sync.h | 4 +++- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/fs/sock.c b/fs/sock.c index 3824d954..60ecbf82 100644 --- a/fs/sock.c +++ b/fs/sock.c @@ -372,7 +372,7 @@ int_t sys_connect(fd_t sock_fd, addr_t sockaddr_addr, uint_t sockaddr_len) { // Wait for acknowledgement that it happened. lock(&peer_lock); while (sock->socket.unix_peer == NULL) - wait_for(&sock->socket.unix_got_peer, &peer_lock, NULL); + wait_for_ignore_signals(&sock->socket.unix_got_peer, &peer_lock, NULL); unlock(&peer_lock); } } diff --git a/kernel/eventfd.c b/kernel/eventfd.c index 12c80865..3ae4484a 100644 --- a/kernel/eventfd.c +++ b/kernel/eventfd.c @@ -29,7 +29,10 @@ static ssize_t eventfd_read(struct fd *fd, void *buf, size_t bufsize) { unlock(&fd->lock); return _EAGAIN; } - wait_for(&fd->cond, &fd->lock, NULL); + if (wait_for(&fd->cond, &fd->lock, NULL)) { + unlock(&fd->lock); + return _EINTR; + } } *(uint64_t *) buf = fd->eventfd.val; @@ -53,7 +56,10 @@ static ssize_t eventfd_write(struct fd *fd, const void *buf, size_t bufsize) { unlock(&fd->lock); return _EAGAIN; } - wait_for(&fd->cond, &fd->lock, NULL); + if (wait_for(&fd->cond, &fd->lock, NULL)) { + unlock(&fd->lock); + return _EINTR; + } } fd->eventfd.val += increment; diff --git a/util/sync.h b/util/sync.h index 176a75a9..ef608f99 100644 --- a/util/sync.h +++ b/util/sync.h @@ -5,6 +5,7 @@ #include #include #include +#include "misc.h" // locks, implemented using pthread @@ -61,7 +62,8 @@ void cond_destroy(cond_t *cond); // Releases the lock, waits for the condition, and reacquires the lock. // Returns _EINTR if waiting stopped because the thread received a signal, // _ETIMEDOUT if waiting stopped because the timout expired, 0 otherwise. -int wait_for(cond_t *cond, lock_t *lock, struct timespec *timeout); +// Will never return _ETIMEDOUT if timeout is NULL. +int must_check wait_for(cond_t *cond, lock_t *lock, struct timespec *timeout); // Same as wait_for, except it will never return _EINTR int wait_for_ignore_signals(cond_t *cond, lock_t *lock, struct timespec *timeout); // Wake up all waiters.