From 27b2fa9ae5d7a3807eea150cef5163931929cc23 Mon Sep 17 00:00:00 2001 From: Omar Polo Date: Fri, 12 Feb 2021 11:27:33 +0000 Subject: [PATCH] don't mmap Before we mmap(2) file for reading, and use a buffer to handle CGI scripts. Turns out, for sequential access over the whole mmap isn't better than our loop on read. This has also the additional advantage that we can use handle_cgi (now handle_copy) for both files and CGI, which is pretty cool. This also fixes a nasty bug where we could hang a connection forever, because we scheduled the wrong type of event (read on POLLOUT and write on POLLIN, it's the other way around!) --- gmid.h | 5 ++- server.c | 104 +++++++++++-------------------------------------------- 2 files changed, 23 insertions(+), 86 deletions(-) diff --git a/gmid.h b/gmid.h index 2fdfc7e..6d339c1 100644 --- a/gmid.h +++ b/gmid.h @@ -171,9 +171,8 @@ struct client { const char *meta; int fd, pfd; DIR *dir; - char sbuf[1024]; /* static buffer */ - void *buf, *i; /* mmap buffer */ - ssize_t len, off; /* mmap/static buffer */ + char sbuf[BUFSIZ]; + ssize_t len, off; struct sockaddr_storage addr; struct vhost *host; /* host she's talking to */ }; diff --git a/server.c b/server.c index 892b0db..b499057 100644 --- a/server.c +++ b/server.c @@ -14,7 +14,6 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -#include #include #include @@ -46,7 +45,6 @@ static inline void reschedule_write(int, struct client*, statefn); static int check_path(struct client*, const char*, int*); static void open_file(struct client*); -static void load_file(struct client*); static void check_for_cgi(struct client*); static void handle_handshake(int, short, void*); static char *strip_path(char*, int); @@ -57,7 +55,6 @@ static void handle_open_conn(int, short, void*); static void start_reply(struct client*, int, const char*); static void handle_start_reply(int, short, void*); static void start_cgi(const char*, const char*, struct client*); -static void send_file(int, short, void*); static void open_dir(struct client*); static void redirect_canonical_dir(struct client*); static void enter_handle_dirlist(int, short, void*); @@ -65,7 +62,7 @@ static void handle_dirlist(int, short, void*); static int read_next_dir_entry(struct client*); static void send_directory_listing(int, short, void*); static void handle_cgi_reply(int, short, void*); -static void handle_cgi(int, short, void*); +static void handle_copy(int, short, void*); static void close_conn(int, short, void*); static void do_accept(int, short, void*); static void handle_sighup(int, short, void*); @@ -277,7 +274,8 @@ open_file(struct client *c) /* fallthrough */ case FILE_EXISTS: - load_file(c); + c->next = handle_copy; + start_reply(c, SUCCESS, mime(c->host, c->iri.path)); return; case FILE_DIRECTORY: @@ -298,27 +296,6 @@ open_file(struct client *c) } } -static void -load_file(struct client *c) -{ - if ((c->len = filesize(c->pfd)) == -1) { - log_err(c, "failed to get file size for %s: %s", - c->iri.path, strerror(errno)); - start_reply(c, TEMP_FAILURE, "internal server error"); - return; - } - - if ((c->buf = mmap(NULL, c->len, PROT_READ, MAP_PRIVATE, - c->pfd, 0)) == MAP_FAILED) { - log_err(c, "mmap: %s: %s", c->iri.path, strerror(errno)); - start_reply(c, TEMP_FAILURE, "internal server error"); - return; - } - c->i = c->buf; - c->next = send_file; - start_reply(c, SUCCESS, mime(c->host, c->iri.path)); -} - /* * the inverse of this algorithm, i.e. starting from the start of the * path + strlen(cgi), and checking if each component, should be @@ -674,39 +651,6 @@ err: fatal("cannot talk to the executor process"); } -static void -send_file(int fd, short ev, void *d) -{ - struct client *c = d; - ssize_t ret, len; - - len = (c->buf + c->len) - c->i; - - while (len > 0) { - switch (ret = tls_write(c->ctx, c->i, len)) { - case -1: - log_err(c, "tls_write: %s", tls_error(c->ctx)); - close_conn(fd, ev, c); - return; - - case TLS_WANT_POLLIN: - reschedule_read(fd, c, &send_file); - return; - - case TLS_WANT_POLLOUT: - reschedule_write(fd, c, &send_file); - return; - - default: - c->i += ret; - len -= ret; - break; - } - } - - close_conn(fd, ev, c); -} - static void open_dir(struct client *c) { @@ -938,7 +882,7 @@ handle_cgi_reply(int fd, short ev, void *d) log_request(c, c->sbuf, c->len); c->off = 0; - handle_cgi(fd, ev, c); + handle_copy(fd, ev, c); return; } @@ -946,39 +890,23 @@ handle_cgi_reply(int fd, short ev, void *d) } static void -handle_cgi(int fd, short ev, void *d) +handle_copy(int fd, short ev, void *d) { struct client *c = d; ssize_t r; while (1) { - if (c->len == 0) { - switch (r = read(c->pfd, c->sbuf, sizeof(c->sbuf))) { - case 0: - goto end; - case -1: - if (errno == EAGAIN || errno == EWOULDBLOCK) { - reschedule_read(c->pfd, c, &handle_cgi); - return; - } - goto end; - default: - c->len = r; - c->off = 0; - } - } - while (c->len > 0) { switch (r = tls_write(c->ctx, c->sbuf + c->off, c->len)) { case -1: goto end; case TLS_WANT_POLLOUT: - reschedule_read(c->fd, c, &handle_cgi); + reschedule_write(c->fd, c, &handle_copy); return; case TLS_WANT_POLLIN: - reschedule_write(c->fd, c, &handle_cgi); + reschedule_read(c->fd, c, &handle_copy); return; default: @@ -987,6 +915,20 @@ handle_cgi(int fd, short ev, void *d) break; } } + + switch (r = read(c->pfd, c->sbuf, sizeof(c->sbuf))) { + case 0: + goto end; + case -1: + if (errno == EAGAIN || errno == EWOULDBLOCK) { + reschedule_read(c->pfd, c, &handle_copy); + return; + } + goto end; + default: + c->len = r; + c->off = 0; + } } end: @@ -1012,9 +954,6 @@ close_conn(int fd, short ev, void *d) tls_free(c->ctx); c->ctx = NULL; - if (c->buf != MAP_FAILED) - munmap(c->buf, c->len); - if (c->pfd != -1) close(c->pfd); @@ -1055,7 +994,6 @@ do_accept(int sock, short et, void *d) c->fd = fd; c->pfd = -1; - c->buf = MAP_FAILED; c->dir = NULL; c->addr = addr;