Redo fetch_read() using non-blocking sockets. This is necessary to

avoid a hang in the SSL case if the server sends a close notification
before we are done reading.  In the non-SSL case, it can provide a
minor (but probably not noticeable) performance improvement for small
transfers.

MFC after:	3 weeks
This commit is contained in:
Dag-Erling Smørgrav 2010-07-28 16:11:22 +00:00
parent 7472b122a9
commit caaffed8f0
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/head/; revision=210568

View File

@ -38,6 +38,7 @@ __FBSDID("$FreeBSD$");
#include <ctype.h>
#include <errno.h>
#include <fcntl.h>
#include <netdb.h>
#include <pwd.h>
#include <stdarg.h>
@ -295,7 +296,8 @@ fetch_connect(const char *host, int port, int af, int verbose)
close(sd);
continue;
}
if (connect(sd, res->ai_addr, res->ai_addrlen) == 0)
if (connect(sd, res->ai_addr, res->ai_addrlen) == 0 &&
fcntl(sd, F_SETFL, O_NONBLOCK) == 0)
break;
close(sd);
}
@ -319,6 +321,7 @@ fetch_connect(const char *host, int port, int af, int verbose)
int
fetch_ssl(conn_t *conn, int verbose)
{
int ret, ssl_err;
#ifdef WITH_SSL
/* Init the SSL library and context */
@ -339,9 +342,13 @@ fetch_ssl(conn_t *conn, int verbose)
return (-1);
}
SSL_set_fd(conn->ssl, conn->sd);
if (SSL_connect(conn->ssl) == -1){
ERR_print_errors_fp(stderr);
return (-1);
while ((ret = SSL_connect(conn->ssl)) == -1) {
ssl_err = SSL_get_error(conn->ssl, ret);
if (ssl_err != SSL_ERROR_WANT_READ &&
ssl_err != SSL_ERROR_WANT_WRITE) {
ERR_print_errors_fp(stderr);
return (-1);
}
}
if (verbose) {
@ -370,6 +377,46 @@ fetch_ssl(conn_t *conn, int verbose)
#endif
}
#define FETCH_READ_WAIT -2
#define FETCH_READ_ERROR -1
#define FETCH_READ_DONE 0
#ifdef WITH_SSL
static ssize_t
fetch_ssl_read(SSL *ssl, char *buf, size_t len)
{
ssize_t rlen;
int ssl_err;
rlen = SSL_read(ssl, buf, len);
if (rlen < 0) {
ssl_err = SSL_get_error(ssl, rlen);
if (ssl_err == SSL_ERROR_WANT_READ ||
ssl_err == SSL_ERROR_WANT_WRITE) {
return (FETCH_READ_WAIT);
} else {
ERR_print_errors_fp(stderr);
return (FETCH_READ_ERROR);
}
}
return (rlen);
}
#endif
static ssize_t
fetch_socket_read(int sd, char *buf, size_t len)
{
ssize_t rlen;
rlen = read(sd, buf, len);
if (rlen < 0) {
if (errno == EAGAIN || (errno == EINTR && fetchRestartCalls))
return (FETCH_READ_WAIT);
else
return (FETCH_READ_ERROR);
}
return (rlen);
}
/*
* Read a character from a connection w/ timeout
@ -390,6 +437,43 @@ fetch_read(conn_t *conn, char *buf, size_t len)
total = 0;
while (len > 0) {
/*
* The socket is non-blocking. Instead of the canonical
* select() -> read(), we do the following:
*
* 1) call read() or SSL_read().
* 2) if an error occurred, return -1.
* 3) if we received data but we still expect more,
* update our counters and loop.
* 4) if read() or SSL_read() signaled EOF, return.
* 5) if we did not receive any data but we're not at EOF,
* call select().
*
* In the SSL case, this is necessary because if we
* receive a close notification, we have to call
* SSL_read() one additional time after we've read
* everything we received.
*
* In the non-SSL case, it may improve performance (very
* slightly) when reading small amounts of data.
*/
#ifdef WITH_SSL
if (conn->ssl != NULL)
rlen = fetch_ssl_read(conn->ssl, buf, len);
else
#endif
rlen = fetch_socket_read(conn->sd, buf, len);
if (rlen == 0) {
break;
} else if (rlen > 0) {
len -= rlen;
buf += rlen;
total += rlen;
continue;
} else if (rlen == FETCH_READ_ERROR) {
return (-1);
}
// assert(rlen == FETCH_READ_WAIT);
while (fetchTimeout && !FD_ISSET(conn->sd, &readfds)) {
FD_SET(conn->sd, &readfds);
gettimeofday(&now, NULL);
@ -413,22 +497,6 @@ fetch_read(conn_t *conn, char *buf, size_t len)
return (-1);
}
}
#ifdef WITH_SSL
if (conn->ssl != NULL)
rlen = SSL_read(conn->ssl, buf, len);
else
#endif
rlen = read(conn->sd, buf, len);
if (rlen == 0)
break;
if (rlen < 0) {
if (errno == EINTR && fetchRestartCalls)
continue;
return (-1);
}
len -= rlen;
buf += rlen;
total += rlen;
}
return (total);
}
@ -548,6 +616,7 @@ fetch_writev(conn_t *conn, struct iovec *iov, int iovcnt)
wlen = writev(conn->sd, iov, iovcnt);
if (wlen == 0) {
/* we consider a short write a failure */
/* XXX perhaps we shouldn't in the SSL case */
errno = EPIPE;
fetch_syserr();
return (-1);