From 695705934172f45e28015658b88afd47ab098f60 Mon Sep 17 00:00:00 2001 From: Jeremy Williams Date: Fri, 10 Feb 2023 17:53:17 -0500 Subject: [PATCH] Move event registration into io_events --- .github/workflows/run-tests.yml | 2 +- Makefile | 2 +- iwyu.imp | 6 +++ src/connection.c | 66 ++++++++++++++++----------------- src/connection.h | 17 +++++---- src/io_events.c | 62 ++++++++++++++++++++++++++----- src/io_events.h | 4 ++ src/parser.rl | 3 -- src/request_util.h | 2 + src/respond.c | 1 + src/server.c | 8 +++- src/server.h | 8 ++++ src/write_socket.c | 42 +-------------------- test/functional/main.c | 4 ++ test/unit/main.c | 2 + test/unit/test_parser.c | 4 +- test/unit/test_read_socket.c | 2 +- test/unit/test_write_socket.c | 2 +- 18 files changed, 136 insertions(+), 101 deletions(-) create mode 100644 iwyu.imp diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index 9d7934f..56a9f6c 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -16,6 +16,6 @@ jobs: - name: make check-format run: make check-format - name: apt-get - run: sudo apt-get install -y apache2-utils ragel + run: sudo apt-get install -y apache2-utils ragel iwyu - name: make run: make diff --git a/Makefile b/Makefile index 642a48b..7e16180 100644 --- a/Makefile +++ b/Makefile @@ -13,7 +13,7 @@ test-functional-cpp: debug debug: build cd build; \ - cmake -DCMAKE_BUILD_TYPE=Debug ..; \ + cmake -DCMAKE_BUILD_TYPE=Debug -DCMAKE_C_INCLUDE_WHAT_YOU_USE="include-what-you-use;-Xiwyu;--mapping_file=$(shell pwd)/iwyu.imp" ..; \ make; \ cd ..; diff --git a/iwyu.imp b/iwyu.imp new file mode 100644 index 0000000..adb1fec --- /dev/null +++ b/iwyu.imp @@ -0,0 +1,6 @@ +[ + { include: ["", "private", "", "public"] }, + { include: ["", "private", "", "public"] }, + { include: ["", "private", "", "public"] }, + { include: ["", "private", "", "public"] } +] diff --git a/src/connection.c b/src/connection.c index 7c89f10..972330d 100644 --- a/src/connection.c +++ b/src/connection.c @@ -1,8 +1,17 @@ #include #include #include +#include #include +#ifdef KQUEUE +#include +#else +#include +#include +#include +#endif + #ifndef HTTPSERVER_IMPL #include "buffer_util.h" #include "common.h" @@ -17,27 +26,24 @@ void _hs_delete_events(http_request_t *request) { kevent(request->server->loop, &ev_set, 1, NULL, 0, NULL); } -void _hs_add_events(http_request_t *request, hs_io_cb_t unused) { +void _hs_add_timer_event(http_request_t *request, hs_io_cb_t unused) { (void)unused; - struct kevent ev_set[2]; - EV_SET(&ev_set[0], request->socket, EVFILT_READ, EV_ADD, 0, 0, request); - EV_SET(&ev_set[1], request->socket, EVFILT_TIMER, EV_ADD | EV_ENABLE, 0, 1000, + struct kevent ev_set; + EV_SET(&ev_set, request->socket, EVFILT_TIMER, EV_ADD | EV_ENABLE, 0, 1000, request); - kevent(request->server->loop, ev_set, 2, NULL, 0, NULL); + kevent(request->server->loop, &ev_set, 1, NULL, 0, NULL); } #else -#include - void _hs_delete_events(http_request_t *request) { epoll_ctl(request->server->loop, EPOLL_CTL_DEL, request->socket, NULL); epoll_ctl(request->server->loop, EPOLL_CTL_DEL, request->timerfd, NULL); close(request->timerfd); } -void _hs_add_events(http_request_t *request, hs_io_cb_t timer_cb) { +void _hs_add_timer_event(http_request_t *request, hs_io_cb_t timer_cb) { request->timer_handler = timer_cb; // Watch for read events @@ -89,30 +95,24 @@ void _hs_init_connection(http_request_t *connection) { _hs_token_array_init(&connection->tokens, 32); } -void hs_accept_connections(http_server_t *server, hs_io_cb_t io_cb, - hs_io_cb_t epoll_timer_cb, - void (*err_responder)(http_request_t *), - int64_t max_mem_usage) { +http_request_t *hs_accept_connection(http_server_t *server, hs_io_cb_t io_cb, + hs_io_cb_t epoll_timer_cb) { + http_request_t *connection = NULL; int sock = 0; - do { - sock = - accept(server->socket, (struct sockaddr *)&server->addr, &server->len); - if (sock > 0) { - http_request_t *connection = - (http_request_t *)calloc(1, sizeof(http_request_t)); - assert(connection != NULL); - connection->socket = sock; - connection->server = server; - connection->timeout = HTTP_REQUEST_TIMEOUT; - connection->handler = io_cb; - int flags = fcntl(sock, F_GETFL, 0); - fcntl(sock, F_SETFL, flags | O_NONBLOCK); - _hs_add_events(connection, epoll_timer_cb); - _hs_init_connection(connection); - connection->state = HTTP_SESSION_READ; - if (connection->server->memused > max_mem_usage) { - err_responder(connection); - } - } - } while (sock > 0); + + sock = accept(server->socket, (struct sockaddr *)&server->addr, &server->len); + + if (sock > 0) { + connection = (http_request_t *)calloc(1, sizeof(http_request_t)); + assert(connection != NULL); + connection->socket = sock; + connection->server = server; + connection->timeout = HTTP_REQUEST_TIMEOUT; + connection->handler = io_cb; + int flags = fcntl(sock, F_GETFL, 0); + fcntl(sock, F_SETFL, flags | O_NONBLOCK); + _hs_add_timer_event(connection, epoll_timer_cb); + _hs_init_connection(connection); + } + return connection; } diff --git a/src/connection.h b/src/connection.h index f49dd10..4509394 100644 --- a/src/connection.h +++ b/src/connection.h @@ -1,16 +1,18 @@ #ifndef HS_CONNECTION_H #define HS_CONNECTION_H +// Forward declarations +struct http_request_s; +struct http_server_s; + #ifdef KQUEUE +struct kevent; typedef void (*hs_io_cb_t)(struct kevent *ev); #else +struct epoll_event; typedef void (*hs_io_cb_t)(struct epoll_event *ev); #endif -// Forward declarations -struct http_request_s; -struct http_server_s; - /* Closes the requests socket and frees its resources. * * Removes all event watchers from the request socket and frees any allocated @@ -38,9 +40,8 @@ void hs_terminate_connection(struct http_request_s *request); * @param max_mem_usage The limit at which err_responder should be called * instead of regular operation. */ -void hs_accept_connections(struct http_server_s *server, hs_io_cb_t io_cb, - hs_io_cb_t epoll_timer_cb, - void (*err_responder)(struct http_request_s *), - int64_t max_mem_usage); +struct http_request_s *hs_accept_connection(struct http_server_s *server, + hs_io_cb_t io_cb, + hs_io_cb_t epoll_timer_cb); #endif diff --git a/src/io_events.c b/src/io_events.c index 3118487..916ad7d 100644 --- a/src/io_events.c +++ b/src/io_events.c @@ -1,6 +1,9 @@ +#include + #ifdef KQUEUE #include #else +#include #include #include #endif @@ -53,8 +56,18 @@ void _hs_write_socket_and_handle_return_code(http_request_t *request) { } } -void _hs_mem_error_responder(http_request_t *request) { - hs_respond_error(request, 503, "Service Unavailable", hs_begin_write); +void _hs_accept_and_begin_request_cycle(http_server_t *server, + hs_io_cb_t on_client_connection_cb, + hs_io_cb_t on_timer_event_cb) { + http_request_t *request = NULL; + while ((request = hs_accept_connection(server, on_client_connection_cb, + on_timer_event_cb))) { + if (server->memused > HTTP_MAX_TOTAL_EST_MEM_USAGE) { + hs_respond_error(request, 503, "Service Unavailable", hs_begin_write); + } else { + hs_begin_read(request); + } + } } #ifdef KQUEUE @@ -79,9 +92,8 @@ void hs_on_kqueue_server_event(struct kevent *ev) { if (ev->filter == EVFILT_TIMER) { hs_generate_date_time(server->date); } else { - hs_accept_connections(server, _hs_on_kqueue_client_connection_event, 0, - _hs_mem_error_responder, - HTTP_MAX_TOTAL_EST_MEM_USAGE); + _hs_accept_and_begin_request_cycle( + server, _hs_on_kqueue_client_connection_event, NULL); } } @@ -108,10 +120,9 @@ void _hs_on_epoll_request_timer_event(struct epoll_event *ev) { } void hs_on_epoll_server_connection_event(struct epoll_event *ev) { - hs_accept_connections((http_server_t *)ev->data.ptr, - _hs_on_epoll_client_connection_event, - _hs_on_epoll_request_timer_event, - _hs_mem_error_responder, HTTP_MAX_TOTAL_EST_MEM_USAGE); + _hs_accept_and_begin_request_cycle((http_server_t *)ev->data.ptr, + _hs_on_epoll_client_connection_event, + _hs_on_epoll_request_timer_event); } void hs_on_epoll_server_timer_event(struct epoll_event *ev) { @@ -125,12 +136,45 @@ void hs_on_epoll_server_timer_event(struct epoll_event *ev) { #endif +void _hs_add_write_event(http_request_t *request) { +#ifdef KQUEUE + struct kevent ev_set[2]; + EV_SET(&ev_set[0], request->socket, EVFILT_WRITE, EV_ADD | EV_CLEAR, 0, 0, + request); + EV_SET(&ev_set[1], request->socket, EVFILT_READ, EV_DISABLE, 0, 0, request); + kevent(request->server->loop, ev_set, 2, NULL, 0, NULL); +#else + struct epoll_event ev; + ev.events = EPOLLOUT | EPOLLET; + ev.data.ptr = request; + epoll_ctl(request->server->loop, EPOLL_CTL_MOD, request->socket, &ev); +#endif +} + void hs_begin_write(http_request_t *request) { request->state = HTTP_SESSION_WRITE; + _hs_add_write_event(request); _hs_write_socket_and_handle_return_code(request); } +void _hs_add_read_event(http_request_t *request) { +#ifdef KQUEUE + // No action needed for kqueue since it's read event stays active. Should + // it be disabled during write? + struct kevent ev_set; + EV_SET(&ev_set, request->socket, EVFILT_READ, EV_ADD | EV_ENABLE, 0, 0, + request); + kevent(request->server->loop, &ev_set, 1, NULL, 0, NULL); +#else + struct epoll_event ev; + ev.events = EPOLLIN | EPOLLET; + ev.data.ptr = request; + epoll_ctl(request->server->loop, EPOLL_CTL_MOD, request->socket, &ev); +#endif +} + void hs_begin_read(http_request_t *request) { request->state = HTTP_SESSION_READ; + _hs_add_read_event(request); _hs_read_socket_and_handle_return_code(request); } diff --git a/src/io_events.h b/src/io_events.h index 9621cd5..195e07d 100644 --- a/src/io_events.h +++ b/src/io_events.h @@ -12,10 +12,14 @@ void hs_begin_read(struct http_request_s *request); #ifdef KQUEUE +struct kevent; + void hs_on_kqueue_server_event(struct kevent *ev); #else +struct epoll_event; + void hs_on_epoll_server_connection_event(struct epoll_event *ev); void hs_on_epoll_server_timer_event(struct epoll_event *ev); diff --git a/src/parser.rl b/src/parser.rl index 2829f92..27373a1 100644 --- a/src/parser.rl +++ b/src/parser.rl @@ -1,8 +1,5 @@ #include #include -#include -#include -#include #ifndef HTTPSERVER_IMPL #include "common.h" diff --git a/src/request_util.h b/src/request_util.h index 2729faa..2af8744 100644 --- a/src/request_util.h +++ b/src/request_util.h @@ -1,6 +1,8 @@ #ifndef HS_REQUEST_UTIL_H #define HS_REQUEST_UTIL_H +#include "common.h" + // http version indicators #define HTTP_1_0 0 #define HTTP_1_1 1 diff --git a/src/respond.c b/src/respond.c index a9cf407..74d2551 100644 --- a/src/respond.c +++ b/src/respond.c @@ -1,5 +1,6 @@ #include #include +#include #include #include #include diff --git a/src/server.c b/src/server.c index dd13e2f..dc1873e 100644 --- a/src/server.c +++ b/src/server.c @@ -1,11 +1,17 @@ +#include #include #include +#include +#include #include +#include #include #ifdef EPOLL -#include +#include #include +#else +#include #endif #ifndef HTTPSERVER_IMPL diff --git a/src/server.h b/src/server.h index 4bca005..a293b4a 100644 --- a/src/server.h +++ b/src/server.h @@ -2,9 +2,17 @@ #define HS_SERVER_H #ifdef KQUEUE + +struct kevent; + typedef void (*hs_evt_cb_t)(struct kevent *ev); + #else + +struct epoll_event; + typedef void (*hs_evt_cb_t)(struct epoll_event *ev); + #endif struct http_request_s; diff --git a/src/write_socket.c b/src/write_socket.c index e62568e..579033c 100644 --- a/src/write_socket.c +++ b/src/write_socket.c @@ -1,11 +1,6 @@ #include -#include +#include #include -#ifdef KQUEUE -#include -#else -#include -#endif #ifndef HTTPSERVER_IMPL #include "buffer_util.h" @@ -26,36 +21,6 @@ void _hs_write_buffer_into_socket(struct hsh_buffer_s *buffer, *bytes_written += bytes; } -void _hs_add_write_event(int event_loop, int request_socket, - void *request_ptr) { -#ifdef KQUEUE - struct kevent ev_set; - EV_SET(&ev_set, request_socket, EVFILT_WRITE, EV_ADD | EV_CLEAR, 0, 0, - request_ptr); - kevent(event_loop, &ev_set, 1, NULL, 0, NULL); -#else - struct epoll_event ev; - ev.events = EPOLLOUT | EPOLLET; - ev.data.ptr = request_ptr; - epoll_ctl(event_loop, EPOLL_CTL_MOD, request_socket, &ev); -#endif -} - -void _hs_add_read_event(int event_loop, int request_socket, void *request_ptr) { -#ifdef KQUEUE - // No action needed for kqueue since it's read event stays active. Should - // it be disabled during write? - (void)event_loop; - (void)request_socket; - (void)request_ptr; -#else - struct epoll_event ev; - ev.events = EPOLLIN | EPOLLET; - ev.data.ptr = request_ptr; - epoll_ctl(event_loop, EPOLL_CTL_MOD, request_socket, &ev); -#endif -} - // Writes response bytes from the buffer out to the socket. // // Runs when we get a socket ready to write event or when initiating an HTTP @@ -75,9 +40,6 @@ enum hs_write_rc_e hs_write_socket(http_request_t *request) { // All bytes of the body were not written and we need to wait until the // socket is writable again to complete the write - _hs_add_write_event(request->server->loop, request->socket, - (void *)request); - request->state = HTTP_SESSION_WRITE; request->timeout = HTTP_REQUEST_TIMEOUT; rc = HS_WRITE_RC_CONTINUE; @@ -93,8 +55,6 @@ enum hs_write_rc_e hs_write_socket(http_request_t *request) { } else { if (HTTP_FLAG_CHECK(request->flags, HTTP_KEEP_ALIVE)) { request->timeout = HTTP_KEEP_ALIVE_TIMEOUT; - _hs_add_read_event(request->server->loop, request->socket, - (void *)request); _hs_buffer_free(&request->buffer, &request->server->memused); } else { rc = HS_WRITE_RC_SUCCESS_CLOSE; diff --git a/test/functional/main.c b/test/functional/main.c index 7015531..b04ceb4 100644 --- a/test/functional/main.c +++ b/test/functional/main.c @@ -1,6 +1,10 @@ #define HTTPSERVER_IMPL #include "../../build/src/httpserver.h" +#include +#include +#include #include +#include #define RESPONSE "Hello, World!" diff --git a/test/unit/main.c b/test/unit/main.c index 3aa3cf7..2445393 100644 --- a/test/unit/main.c +++ b/test/unit/main.c @@ -1,3 +1,5 @@ +#include + #include "munit.h" #include "test_parser.h" #include "test_read_socket.h" diff --git a/test/unit/test_parser.c b/test/unit/test_parser.c index 4997554..dbf108b 100644 --- a/test/unit/test_parser.c +++ b/test/unit/test_parser.c @@ -1,9 +1,9 @@ #include "munit.h" -#include +#include +#include #include "common.h" #include "parser.h" -#include "../debugbreak.h" void setup_buffer_and_parser(struct hsh_parser_s *parser, struct hsh_buffer_s *buffer, char const *req_str) { diff --git a/test/unit/test_read_socket.c b/test/unit/test_read_socket.c index 8afd501..ea67d95 100644 --- a/test/unit/test_read_socket.c +++ b/test/unit/test_read_socket.c @@ -1,10 +1,10 @@ #include #include +#include #include "munit.h" #include "read_socket.h" #include "common.h" -#include "../debugbreak.h" void token_array_init(struct hs_token_array_s *array, int capacity) { array->buf = diff --git a/test/unit/test_write_socket.c b/test/unit/test_write_socket.c index 802abd7..4666980 100644 --- a/test/unit/test_write_socket.c +++ b/test/unit/test_write_socket.c @@ -1,9 +1,9 @@ #include +#include #include "munit.h" #include "write_socket.h" #include "common.h" -#include "../debugbreak.h" enum hs_test_write_mode_e { HS_TEST_WRITE_SUCCESS,