Move event registration into io_events

.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

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

iwyu.imp

@@ -0,0 +1,6 @@
+[
+  { include: ["<bits/types/struct_itimerspec.h>", "private", "<time.h>", "public"] },
+  { include: ["<sys/fcntl.h>", "private", "<fcntl.h>", "public"] },
+  { include: ["<sys/signal.h>", "private", "<signal.h>", "public"] },
+  { include: ["<sys/errno.h>", "private", "<errno.h>", "public"] }
+]

src/connection.c

@@ -1,8 +1,17 @@
 #include <assert.h>
 #include <fcntl.h>
 #include <stdlib.h>
+#include <sys/socket.h>
 #include <unistd.h>
 
+#ifdef KQUEUE
+#include <sys/event.h>
+#else
+#include <sys/epoll.h>
+#include <sys/timerfd.h>
+#include <time.h>
+#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 <sys/timerfd.h>
-
 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;
 }

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

src/io_events.c

@@ -1,6 +1,9 @@
+#include <stdlib.h>
+
 #ifdef KQUEUE
 #include <sys/event.h>
 #else
+#include <stdint.h>
 #include <sys/epoll.h>
 #include <unistd.h>
 #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);
 }

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);
 

src/parser.rl

@@ -1,8 +1,5 @@
 #include <string.h>
 #include <stdlib.h>
-#include <string.h>
-#include <stdint.h>
-#include <assert.h>
 
 #ifndef HTTPSERVER_IMPL
 #include "common.h"

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

src/respond.c

@@ -1,5 +1,6 @@
 #include <assert.h>
 #include <stdarg.h>
+#include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>

src/server.c

@@ -1,11 +1,17 @@
+#include <arpa/inet.h>
 #include <assert.h>
 #include <fcntl.h>
+#include <netinet/in.h>
+#include <signal.h>
 #include <stdlib.h>
+#include <sys/socket.h>
 #include <time.h>
 
 #ifdef EPOLL
-#include <signal.h>
+#include <sys/epoll.h>
 #include <sys/timerfd.h>
+#else
+#include <sys/event.h>
 #endif
 
 #ifndef HTTPSERVER_IMPL

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;