keepalive timer also closes a freshly dialed pooled connection during the HTTP/1.1 TLS upgrade

Offen
#938 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen

Maintainer antworten meist innerhalb von 1 Tag

Dieses Issue hat noch niemand übernommen.

Bewertung

Schwierigkeit
4/5
Geschätzter Aufwand
3-5 Tage
Anfängerfreundlichkeit
58/100
Issue-Typ
Bug
Klarheit
Klar beschrieben
Aktivitätsstatus
Aktiv
Tech-Stack
erlang
Bereich
backend, networking

Rechercherichtung

Start with the fresh-dial path in hackney_pool.erl:573-587 and 1005-1024, then trace connected and upgrade_to_ssl in hackney_conn.erl:845-869 and 996-1013. Compare the HTTP/1.1 and HTTP/2 upgrade branches, and run the pool-enabled HTTP/1 TLS-upgrade tests with a delayed handshake; done means a fresh connection is not closed by the keepalive timer during that handshake.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Beschreibung

hackney 4.7.4 (also current master).

#934 disarms the keepalive timer in the is_ready probe, which covers a reused pooled connection. A fresh dial does not reach is_ready:

  • handle_call({checkout, ...}) takes the none -> start_connection branch (hackney_pool.erl:573-587).
  • The new conn arms idle_timeout = keepalive_timeout on entry to connected (hackney_conn.erl:845-869). keepalive_timeout is capped at 2000 ms by min/2 in hackney_pool.erl:476-482 (an infinity value also collapses to 2000 because of Erlang term order), and start_connection/6 does not take a per-checkout override (hackney_pool.erl:1005-1024).
  • The HTTP/1.1 branch of upgrade_to_ssl returns keep_state and leaves that state_timeout running (hackney_conn.erl:1012-1013). The HTTP/2 branch cancels it via init_h2_after_upgrade (:2995-3007).
  • ssl:connect/3 at :996 blocks the gen_statem. If the TLS handshake takes longer than the remaining timer, the timer fires (:915-917), the conn goes to closed, and the caller's {request, ...} hits the catch-all and gets {error, invalid_state} (:2005-2006 on 4.7.4; {error, closed} on master after #936). No byte of the request has been written at that point.

Reproduced with a TCP proxy in front of api.stripe.com that delays the ClientHello: a 1900 ms delay succeeds, a 2200 ms delay fails with invalid_state, with pool enabled and protocols: [http1]. With prewarm_count: 0 the fresh-dial path is the only path, so the failure rate is simply the fraction of handshakes slower than ~2 s.

Suggested fix: arm the idle timer in connected(enter) only when owner =:= pool_pid (prewarm conns set owner => PoolPid at hackney_pool.erl:1367; fresh-dial conns set owner => Requester at :1024), cancel it in set_owner when the new owner is not the pool, and add the HTTP/2 branch's CancelIdle action to the HTTP/1.1 branch of upgrade_to_ssl.

Related: #916 (stuck in upgrade_to_ssl), #922 (handshake bounded by connect_timeout), #934, #936.

Vorherrschende Sprache
Erlang
Sterne
1.4k
Forks
458
Ø Merge
11 Std. 32 Min.
Gemergte PRs (30 T.)
47

Entwicklungsumgebung

Erste Schritte

  1. Lesen Sie das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreiben Sie ins Issue, dass Sie es übernehmen — das erspart doppelte Arbeit.
  3. Forken Sie das Repository und arbeiten Sie in einem Branch.
  4. Öffnen Sie einen Pull Request, der die Issue-Nummer nennt.

Mehr aus benoitc/hackney

Alle Issues in benoitc/hackney

Ähnliche Issues

Weitere Issues zu Backend & API Design

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.