commit 44514058b96440eaa1b364b915860ae372207ca6 Author: Nick Mathewson nickm@torproject.org Date: Wed Mar 15 11:53:01 2017 -0400
Correctly handle fd-drain errors on windows workqueues
Windows doesn't let you check the socket error for a socket with WSAGetLastError() and getsockopt(SO_ERROR). But getsockopt(SO_ERROR) clears the error on the socket, so you can't call it more than once per error.
When we introduced recv_ni to help drain alert sockets, back in 0.2.6.3-alpha, we had the failure path for recv_ni call getsockopt() twice, though: once to check for EINTR and one to check for EAGAIN. Of course, we never got the eagain, so we treated it as an error, and warned about: "No error".
The fix here is to have these functions return -errno on failure.
Fixes bug 21540; bugfix on 0.2.6.3-alpha. --- changes/bug21540 | 4 +++ src/common/compat_threads.c | 68 ++++++++++++++++++++++++++++++--------------- src/common/workqueue.c | 5 ++-- 3 files changed, 52 insertions(+), 25 deletions(-)
diff --git a/changes/bug21540 b/changes/bug21540 new file mode 100644 index 0000000..0cf684b --- /dev/null +++ b/changes/bug21540 @@ -0,0 +1,4 @@ + o Minor bugfixes (windows, relay): + - Resolve "Failure from drain_fd: No error" warnings on Windows + relays. Fixes bug 21540; bugfix on 0.2.6.3-alpha. + diff --git a/src/common/compat_threads.c b/src/common/compat_threads.c index f480906..a25d9a7 100644 --- a/src/common/compat_threads.c +++ b/src/common/compat_threads.c @@ -94,51 +94,73 @@ in_main_thread(void) }
#if defined(HAVE_EVENTFD) || defined(HAVE_PIPE) -/* As write(), but retry on EINTR */ +/* As write(), but retry on EINTR, and return the negative error code on + * error. */ static int write_ni(int fd, const void *buf, size_t n) { int r; again: r = (int) write(fd, buf, n); - if (r < 0 && errno == EINTR) - goto again; + if (r < 0) { + if (errno == EINTR) + goto again; + else + return -errno; + } return r; } -/* As read(), but retry on EINTR */ +/* As read(), but retry on EINTR, and return the negative error code on error. + */ static int read_ni(int fd, void *buf, size_t n) { int r; again: r = (int) read(fd, buf, n); - if (r < 0 && errno == EINTR) - goto again; + if (r < 0) { + if (errno == EINTR) + goto again; + else + return -errno; + } return r; } #endif
-/** As send(), but retry on EINTR. */ +/** As send(), but retry on EINTR, and return the negative error code on + * error. */ static int send_ni(int fd, const void *buf, size_t n, int flags) { int r; again: r = (int) send(fd, buf, n, flags); - if (r < 0 && ERRNO_IS_EINTR(tor_socket_errno(fd))) - goto again; + if (r < 0) { + int error = tor_socket_errno(fd); + if (ERRNO_IS_EINTR(error)) + goto again; + else + return -error; + } return r; }
-/** As recv(), but retry on EINTR. */ +/** As recv(), but retry on EINTR, and return the negative error code on + * error. */ static int recv_ni(int fd, void *buf, size_t n, int flags) { int r; again: r = (int) recv(fd, buf, n, flags); - if (r < 0 && ERRNO_IS_EINTR(tor_socket_errno(fd))) - goto again; + if (r < 0) { + int error = tor_socket_errno(fd); + if (ERRNO_IS_EINTR(error)) + goto again; + else + return -error; + } return r; }
@@ -149,7 +171,7 @@ eventfd_alert(int fd) { uint64_t u = 1; int r = write_ni(fd, (void*)&u, sizeof(u)); - if (r < 0 && errno != EAGAIN) + if (r < 0 && -r != EAGAIN) return -1; return 0; } @@ -160,8 +182,8 @@ eventfd_drain(int fd) { uint64_t u = 0; int r = read_ni(fd, (void*)&u, sizeof(u)); - if (r < 0 && errno != EAGAIN) - return -1; + if (r < 0 && -r != EAGAIN) + return r; return 0; } #endif @@ -172,8 +194,8 @@ static int pipe_alert(int fd) { ssize_t r = write_ni(fd, "x", 1); - if (r < 0 && errno != EAGAIN) - return -1; + if (r < 0 && -r != EAGAIN) + return r; return 0; }
@@ -188,7 +210,7 @@ pipe_drain(int fd) r = read_ni(fd, buf, sizeof(buf)); } while (r > 0); if (r < 0 && errno != EAGAIN) - return -1; + return -errno; /* A value of r = 0 means EOF on the fd so successfully drained. */ return 0; } @@ -200,13 +222,13 @@ static int sock_alert(tor_socket_t fd) { ssize_t r = send_ni(fd, "x", 1, 0); - if (r < 0 && !ERRNO_IS_EAGAIN(tor_socket_errno(fd))) - return -1; + if (r < 0 && !ERRNO_IS_EAGAIN(-r)) + return r; return 0; }
/** Drain all the input from a socket <b>fd</b>, and ignore it. Return 0 on - * success, -1 on error. */ + * success, -errno on error. */ static int sock_drain(tor_socket_t fd) { @@ -215,8 +237,8 @@ sock_drain(tor_socket_t fd) do { r = recv_ni(fd, buf, sizeof(buf), 0); } while (r > 0); - if (r < 0 && !ERRNO_IS_EAGAIN(tor_socket_errno(fd))) - return -1; + if (r < 0 && !ERRNO_IS_EAGAIN(-r)) + return r; /* A value of r = 0 means EOF on the fd so successfully drained. */ return 0; } diff --git a/src/common/workqueue.c b/src/common/workqueue.c index e1fb663..ec6e257 100644 --- a/src/common/workqueue.c +++ b/src/common/workqueue.c @@ -510,12 +510,13 @@ replyqueue_get_socket(replyqueue_t *rq) void replyqueue_process(replyqueue_t *queue) { - if (queue->alert.drain_fn(queue->alert.read_fd) < 0) { + int r = queue->alert.drain_fn(queue->alert.read_fd); + if (r < 0) { //LCOV_EXCL_START static ratelim_t warn_limit = RATELIM_INIT(7200); log_fn_ratelim(&warn_limit, LOG_WARN, LD_GENERAL, "Failure from drain_fd: %s", - tor_socket_strerror(tor_socket_errno(queue->alert.read_fd))); + tor_socket_strerror(-r)); //LCOV_EXCL_STOP }