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.
siloam72761 · Apr 19, 2020 · 103257c · 103257c
1 parent 95f6413
commit 103257c
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 4 deletions.
diff --git 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
@@ -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
@@ -5,6 +5,7 @@
 #include <pthread.h>
 #include <stdbool.h>
 #include <setjmp.h>
+#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.