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.
This commit is contained in:
Nick Mathewson 2017-03-15 11:53:01 -04:00
parent d642ceb8df
commit 44514058b9
3 changed files with 52 additions and 25 deletions

4
changes/bug21540 Normal file
View File

@ -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.

View File

@ -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;
}

View File

@ -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
}