53565fbfd6 — curl

Subject

conncache: connection healthiness fix

Binary

No binary (build failed)

Architecture & Compiler Flags

Architecture: x86_64-linux-gnu Compiler: gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

—

Build Log

(log yok)

Commit Diff GitHub →

lib/conncache.c +29 -40 lib/conncache.h +5 -6 lib/http2.c +2 -2 lib/url.c +6 -5

lib/conncache.c+29 -40

@@ -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

lib/conncache.h+5 -6

@@ -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 */

lib/http2.c+2 -2

@@ -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 */

lib/url.c+6 -5

@@ -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