Fix timeout behavior with persistent connections.

We'd either never time out clients (if the expiry check happened
while they were receiving a request) or time them out too soon
(based on the connection time instead of the time of last request end).

diff --git a/client.cpp b/client.cpp
index 05b8e7d88fcd0b622e2386e84b83abedf0687a24..23892ac19038406e584241a0ee462583c9447c0b 100644 (file)
--- a/client.cpp
+++ b/client.cpp
@@ -20,11 +20,6 @@ Client::Client(int sock)
 {
        request.reserve(1024);
 
-       if (clock_gettime(CLOCK_MONOTONIC_COARSE, &connect_time) == -1) {
-               log_perror("clock_gettime(CLOCK_MONOTONIC_COARSE)");
-               return;
-       }
-
        // Find the remote address, and convert it to ASCII.
        sockaddr_in6 addr;
        socklen_t addr_len = sizeof(addr);

diff --git a/client.h b/client.h
index c18df4aaa06eed0a49f1241240c92581fd64615f..858197fdcc457dd19a73d5ffb66af50bff5cf5f2 100644 (file)
--- a/client.h
+++ b/client.h
@@ -42,7 +42,7 @@ struct Client {
        // The file descriptor associated with this socket.
        int sock;
 
-       // When the client connected, in terms of CLOCK_MONOTONIC_COARSE.
+       // When the client connected (or went into keepalive), in terms of CLOCK_MONOTONIC_COARSE.
        timespec connect_time;
 
        // Some information only used for logging.

diff --git a/server.cpp b/server.cpp
index ccac946726bf9710946ebb3a5612386ab0690e62..78273b00eb7f3b9b49938c376eba4aeb92fe4300 100644 (file)
--- a/server.cpp
+++ b/server.cpp
@@ -210,16 +210,7 @@ void Server::add_client(int sock, Acceptor *acceptor)
        assert(inserted.second == true);  // Should not already exist.
        Client *client_ptr = &inserted.first->second;
 
-       // Connection timestamps must be nondecreasing. I can't find any guarantee
-       // that even the monotonic clock can't go backwards by a small amount
-       // (think switching between CPUs with non-synchronized TSCs), so if
-       // this actually should happen, we hack around it by fudging
-       // connect_time.
-       if (!clients_ordered_by_connect_time.empty() &&
-           is_earlier(client_ptr->connect_time, clients_ordered_by_connect_time.back().first)) {
-               client_ptr->connect_time = clients_ordered_by_connect_time.back().first;
-       }
-       clients_ordered_by_connect_time.push(make_pair(client_ptr->connect_time, sock));
+       start_client_timeout_timer(client_ptr);
 
        // Start listening on data from this socket.
        epoll_event ev;
@@ -302,6 +293,24 @@ void Server::add_client_from_serialized(const ClientProto &client, const vector<
        }
 }
 
+void Server::start_client_timeout_timer(Client *client)
+{
+       // Connection timestamps must be nondecreasing. I can't find any guarantee
+       // that even the monotonic clock can't go backwards by a small amount
+       // (think switching between CPUs with non-synchronized TSCs), so if
+       // this actually should happen, we hack around it by fudging
+       // connect_time.
+       if (clock_gettime(CLOCK_MONOTONIC_COARSE, &client->connect_time) == -1) {
+               log_perror("clock_gettime(CLOCK_MONOTONIC_COARSE)");
+       } else {
+               if (!clients_ordered_by_connect_time.empty() &&
+                   is_earlier(client->connect_time, clients_ordered_by_connect_time.back().first)) {
+                       client->connect_time = clients_ordered_by_connect_time.back().first;
+               }
+               clients_ordered_by_connect_time.push(make_pair(client->connect_time, client->sock));
+       }
+}
+
 int Server::lookup_stream_by_url(const string &url) const
 {
        const auto stream_url_it = stream_url_map.find(url);
@@ -1323,6 +1332,7 @@ bool Server::more_requests(Client *client)
        client->header_or_short_response_holder.clear();
        client->header_or_short_response_ref.reset();
        client->header_or_short_response_bytes_sent = 0;
+       start_client_timeout_timer(client);
 
        change_epoll_events(client, EPOLLIN | EPOLLET | EPOLLRDHUP);  // No TLS handshake, so no EPOLLOUT needed.
 

diff --git a/server.h b/server.h
index 4ec908b039c86b24db7b8b252d27a71515a71c60..cfb6b2e13680409314ba444f68dc023666d8de31 100644 (file)
--- a/server.h
+++ b/server.h
@@ -201,6 +201,10 @@ private:
        void skip_lost_data(Client *client);
 
        void add_client(int sock, Acceptor *acceptor);
+
+       // Mark that a client just went into READING_REQUEST state, so we should
+       // note the current time of day and then put it into <clients_ordered_by_connect_time>.
+       void start_client_timeout_timer(Client *client);
 };
 
 #endif  // !defined(_SERVER_H)