conncache: connection healthiness fix

Update the `lastchecked` timestamp on connection health checks when
successful to prevent repeated recalcs for a second.

Rename `seems_dead` to `seems_healthy` because the world is already
depressing enough.

Consider pending input on connection only unhealthy when the connection
has no transfers and is not multiplexed.

Closes #22412

diff --git a/lib/conncache.c b/lib/conncache.c
index 897b41cc42514b92b3ac62b01733dcb7e9095f99..11ef03343557609f72ff5338c556c365a3e1b621 100644 (file)
--- a/lib/conncache.c
+++ b/lib/conncache.c
@@ -702,7 +702,6 @@ void Curl_conn_terminate(struct Curl_easy *data,
 }
 
 struct cpool_reaper_ctx {
-  size_t checked;
   size_t reaped;
   struct curltime now;
 };
@@ -711,17 +710,15 @@ static int cpool_reap_dead_cb(struct Curl_easy *data,
                               struct connectdata *conn, void *param)
 {
   struct cpool_reaper_ctx *reaper = param;
-  bool terminate = !CONN_INUSE(conn) && conn->bits.no_reuse;
 
-  if(!terminate) {
-    reaper->checked++;
-    terminate = Curl_cpool_conn_seems_dead(conn, data, &reaper->now);
-  }
-  if(terminate) {
-    /* stop the iteration here, pass back the connection that was pruned */
-    reaper->reaped++;
-    Curl_conn_terminate(data, conn, FALSE);
-    return 1;
+  if(!CONN_INUSE(conn)) {
+    if(conn->bits.no_reuse || conn->bits.close ||
+       !Curl_cpool_conn_seems_healthy(conn, data, &reaper->now)) {
+      /* terminate conn and stop the iteration */
+      reaper->reaped++;
+      Curl_conn_terminate(data, conn, FALSE);
+      return 1;
+    }
   }
   return 0; /* continue iteration */
 }
@@ -906,49 +903,41 @@ static bool cpool_conn_maxage(struct Curl_easy *data,
   return FALSE;
 }
 
-/*
- * Return TRUE iff the given connection is considered dead.
- */
-bool Curl_cpool_conn_seems_dead(struct connectdata *conn,
-                                struct Curl_easy *data,
-                                const struct curltime *pnow)
+bool Curl_cpool_conn_seems_healthy(struct connectdata *conn,
+                                   struct Curl_easy *data,
+                                   const struct curltime *pnow)
 {
-  bool input_pending = FALSE;
-  bool dead = FALSE;
+  bool healthy = TRUE;
 
   DEBUGASSERT(!data->conn);
-  /* The check only makes sense only if the connection is not in use */
-  if(CONN_INUSE(conn))
+  if(!CONN_INUSE(conn) && cpool_conn_maxage(data, conn, pnow)) /* too old? */
     return FALSE;
-  else if(cpool_conn_maxage(data, conn, pnow)) /* too old? */
-    return TRUE;
   else if(curlx_ptimediff_ms(pnow, &conn->lastchecked) < 1000)
-    return FALSE;
+    return TRUE;
   else if(conn->scheme->run->connection_is_dead) {
     Curl_attach_connection(data, conn);
-    dead = conn->scheme->run->connection_is_dead(data, conn);
+    healthy = !conn->scheme->run->connection_is_dead(data, conn);
     Curl_detach_connection(data);
   }
   else {
+    bool input_pending = FALSE;
+
     Curl_attach_connection(data, conn);
-    dead = !Curl_conn_is_alive(data, conn, &input_pending);
+    healthy = Curl_conn_is_alive(data, conn, &input_pending);
     Curl_detach_connection(data);
+    if(healthy && input_pending &&
+       !CONN_INUSE(conn) && !Curl_conn_is_multiplex(conn, FIRSTSOCKET)) {
+      /* Non-multiplexed connections without attached transfers should
+       * not have input pending. The input might be a TLS Notify Close,
+       * for all we know. */
+      DEBUGF(infof(data, "connection has no transfer but input, not healthy"));
+      healthy = FALSE;
+    }
   }
 
-  if(input_pending) {
-    /* For reuse, we want a "clean" connection state. This includes
-     * that we expect - in general - no waiting input data. Input
-     * waiting might be a TLS Notify Close, for example. We reject
-     * that.
-     * For protocols where data from other end may arrive at
-     * any time (HTTP/2 PING for example), the protocol handler needs
-     * to install its own `connection_check` callback.
-     */
-    DEBUGF(infof(data, "connection has input pending, not reusable"));
-    dead = TRUE;
-  }
-
-  return dead;
+  if(healthy)
+    conn->lastchecked = *pnow;
+  return healthy;
 }
 
 #if 0

diff --git a/lib/conncache.h b/lib/conncache.h
index 6798a3b8b3e7d5035f8dbbb835ad087349b75479..4924c038b36f13486ca4f77ed25eb6a1e712b0e1 100644 (file)
--- a/lib/conncache.h
+++ b/lib/conncache.h
@@ -156,11 +156,10 @@ void Curl_cpool_do_locked(struct Curl_easy *data,
 /* Close all unused connections, prevent reuse of existing ones. */
 void Curl_cpool_nw_changed(struct Curl_easy *data);
 
-/**
- * Return TRUE iff the given connection is considered dead.
- */
-bool Curl_cpool_conn_seems_dead(struct connectdata *conn,
-                                struct Curl_easy *data,
-                                const struct curltime *pnow);
+/* Return TRUE iff the given connection is considered healthy, e.g.
+ * usable for more transfers. */
+bool Curl_cpool_conn_seems_healthy(struct connectdata *conn,
+                                   struct Curl_easy *data,
+                                   const struct curltime *pnow);
 
 #endif /* HEADER_CURL_CONNCACHE_H */

diff --git a/lib/http2.c b/lib/http2.c
index 8d4a93cb58b0d1403af7e2e566b973b388d14752..b8733d8ec12792471f53c893ee12ad1c02ef2322 100644 (file)
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -545,9 +545,9 @@ static bool http2_connisalive(struct Curl_cfilter *cf, struct Curl_easy *data,
 
     *input_pending = FALSE;
     result = Curl_cf_recv_bufq(cf->next, data, &ctx->inbufq, 0, &nread);
+    CURL_TRC_CF(data, cf, "connisalive, recv pending input -> %d, %zu",
+                (int)result, nread);
     if(!result) {
-      CURL_TRC_CF(data, cf, "%zu bytes stray data read before trying "
-                  "h2 connection", nread);
       result = h2_process_pending_input(cf, data);
       if(result)
         /* immediate error, considered dead */

diff --git a/lib/url.c b/lib/url.c
index 4cb25350338ccb614b59f72356e231ed8817f3b4..5cc5ab7ce9624b01a7fcca9487207e8189eb30e8 100644 (file)
--- a/lib/url.c
+++ b/lib/url.c
@@ -1063,8 +1063,10 @@ static bool url_match_conn(struct connectdata *conn, void *userdata)
   if(!url_match_multiplex_limits(conn, m))
     return FALSE;
 
-  if(Curl_cpool_conn_seems_dead(conn, m->data, &m->now)) {
-    /* remove and disconnect. */
+  /* If we are going to pick an idle connection, do an extra
+   * health check before we reuse it. */
+  if(!CONN_INUSE(conn) &&
+     !Curl_cpool_conn_seems_healthy(conn, m->data, &m->now)) {
     infof(m->data, "Connection %" FMT_OFF_T " seems to be dead, terminating",
           conn->connection_id);
     Curl_conn_terminate(m->data, conn, FALSE);
@@ -1117,6 +1119,8 @@ static bool url_attach_existing(struct Curl_easy *data,
   bool success;
 
   DEBUGASSERT(!data->conn);
+  Curl_cpool_prune_dead(data);
+
   memset(&match, 0, sizeof(match));
   match.data = data;
   match.needle = needle;
@@ -2283,9 +2287,6 @@ static CURLcode url_find_or_create_conn(struct Curl_easy *data)
   if(result)
     goto out;
 
-  /* Get rid of any dead connections so limit are easier kept. */
-  Curl_cpool_prune_dead(data);
-
   /*************************************************************
    * Reuse of existing connection is not allowed when
    * - connect_only is set or