jeremycw / httpserver.h

Single header library for writing non-blocking HTTP servers in C
MIT License
1.78k stars 143 forks source link

Broken connection after interrupted write #59

Closed suyjuris closed 1 year ago

suyjuris commented 2 years ago

Thank you for writing this library! I find it quite useful.

I encountered an issue where the server sometimes does not seem to receive requests. (This is on an application I am developing, and the requests originate from Firefox.) Digging a bit deeper, I think the following code is able to reliably reproduce the issue:

#include <unistd.h>
#undef _POSIX_C_SOURCE

#define HTTPSERVER_IMPL
#include "httpserver.h"

char buf[1024 * 1024 * 8];

void handle_request(struct http_request_s* request) {
    puts("Received request, sending answer");
    struct http_response_s* response = http_response_init();
    http_response_status(response, 200);
    http_response_header(response, "Content-Type", "text/plain");
    http_response_body(response, buf, sizeof(buf));
    http_respond(request, response);
}

void read_n(int sock, ssize_t n) {
    // Read in chunks. This is off-by-one intentionally, to also read the headers.
    int wait = 1;
    while (n >= 0) {
        n -= read(sock, buf, 4096);
        if (wait) usleep(1000);
        wait = 0;
    }
}

int main(int argc, char** argv) {
    memset(buf, '.', sizeof(buf));
    for (int i = 127; i < sizeof(buf); i+= 128) {
        buf[i] = '\n';
    }

    if (argc == 1) {
        struct http_server_s* server = http_server_init(8080, handle_request);
        puts("Starting server");
        http_server_listen(server);
    } else {
        int sock = socket(AF_INET, SOCK_STREAM, 0);

        // Connect to 127.0.0.1:8080
        struct sockaddr_in addr;
        memset(&addr, 0, sizeof(addr));
        addr.sin_family = AF_INET;
        addr.sin_port = 0x901f;
        addr.sin_addr.s_addr = 0x0100007f;
        connect(sock, (struct sockaddr*)&addr, sizeof(addr));

        char req[] = "GET / HTTP/1.1\r\nHost: localhost\r\n\r\n";

        puts("Sending first request");
        write(sock, req, sizeof(req));
        puts("Sent.");
        read_n(sock, sizeof(buf));
        puts("Received answer");

        puts("Sending second request");
        write(sock, req, sizeof(req));
        puts("Sent.");
        read_n(sock, sizeof(buf));
        puts("Received answer");
    }
}

To reproduce, compile this code and run two instances: the first without arguments, and the second with one argument (it does not matter which). The first instance starts the server, while the second tries to send two simple requests. On my machine, only the first of these requests is handled. I believe that the following happens:

  1. The first request is sent.
  2. The server receives the first request, and returns a large(-ish) response (8 MiB).
  3. The client reads this response, but waits after reading a small chunk of it.
  4. This interrupts the write on the server and triggers the logic in hs_write_response, calling hs_add_write_event.
  5. The client continues to read the rest of the response.
  6. The server completes its handling of the request and re-initialises the session for future requests (third case in hs_write_response).
  7. The client sends another request.
  8. Nothing happens, as the call to hs_add_write_event cleared the EPOLLIN flag

Based on the above, I think that restoring the EPOLLIN flag at step 6 should fix this issue, and indeed, that is what happens on my machine. I will be making a pull request with the code shortly.

jeremycw commented 1 year ago

Thanks you for finding this. I've incorporated the fix into the latest release.