Skip to content

bitreq: Fix connection persistence, pool refresh#661

Closed
tnull wants to merge 5 commits into
rust-bitcoin:masterfrom
tnull:2026-07-bitreq-persistence-pool
Closed

bitreq: Fix connection persistence, pool refresh#661
tnull wants to merge 5 commits into
rust-bitcoin:masterfrom
tnull:2026-07-bitreq-persistence-pool

Conversation

@tnull

@tnull tnull commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Based on #660.
Closes #659.

Honor HTTP persistence rules

Reuse HTTP/1.1 connections unless either endpoint requests
closure, and require explicit keep-alive only for HTTP/1.0. This
allows compliant servers to omit redundant keep-alive headers while
still preventing reuse after a close option.
Refresh async pool entries on reuse 

A cache hit must make its connection the most recently used entry.
Without that refresh, inserting another origin can evict an actively
reused connection and retain an older idle one.

tnull added 3 commits July 11, 2026 15:21
Exercise the HTTP/1.1 wire grammar so request generation and
response parsing gaps are visible before behavior is changed.

Co-Authored-By: HAL 9000
Cover response length and chunk decoding rules so incomplete or
ambiguous messages cannot pass unnoticed.

Co-Authored-By: HAL 9000
Cover persistence, closure, and pipelining rules so connection
reuse can be corrected against explicit expectations.

Refs: rust-bitcoin#659

Co-Authored-By: HAL 9000

@TheBlueMatt TheBlueMatt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, changes in this PR LGTM, had one question in the dep PR though.

Comment thread bitreq/src/connection.rs Outdated
Comment thread bitreq/src/client.rs Outdated
tnull added 2 commits July 20, 2026 09:50
Reuse HTTP/1.1 connections unless either endpoint requests
closure, and require explicit keep-alive only for HTTP/1.0. This
allows compliant servers to omit redundant keep-alive headers while
still preventing reuse after a close option.

Fixes rust-bitcoin#659

Co-Authored-By: HAL 9000
A cache hit must make its connection the most recently used entry.
Without that refresh, inserting another origin can evict an actively
reused connection and retain an older idle one.

Co-Authored-By: HAL 9000
@tnull
tnull force-pushed the 2026-07-bitreq-persistence-pool branch from b925c3e to f46ab3a Compare July 20, 2026 07:50
@tnull

tnull commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator Author

Force-pushed with the following changes:

diff --git a/bitreq/src/client.rs b/bitreq/src/client.rs
index f2a9bb25..d71c349c 100644
--- a/bitreq/src/client.rs
+++ b/bitreq/src/client.rs
@@ -69,5 +69,6 @@ impl Client {
             let mut state = self.r#async.lock().unwrap();

-            if let Some(conn) = state.connections.get(&owned_key).cloned() {
+            if let Some(conn) = state.connections.get(&owned_key) {
+                let conn = Arc::clone(conn);
                 state.lru_order.retain(|key| key != &owned_key);
                 state.lru_order.push_back(owned_key.clone());
diff --git a/bitreq/src/connection.rs b/bitreq/src/connection.rs
index 1fe7b92c..3b28bad4 100644
--- a/bitreq/src/connection.rs
+++ b/bitreq/src/connection.rs
@@ -587,5 +587,5 @@ impl AsyncConnection {
                 .await?;

-                let connection = response.headers.get("connection").map(String::as_str);
+                let connection = response.headers.get("connection");
                 let received_close =
                     connection.is_some_and(|value| has_connection_option(value, "close"));

@tnull
tnull requested a review from TheBlueMatt July 20, 2026 07:51
@tcharding

Copy link
Copy Markdown
Member

Please note development has migrated to https://git.rust-bitcoin.org/rust-bitcoin/corepc. Any further comments or pushes here on github may be ignored or lost. Closing since I know @tnull is ok with the migration.

@tcharding tcharding closed this Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bitreq::Client only reuses an HTTP/1.1 connection when the response explicitly contains Connection: keep-alive

3 participants