Guard get_state liveness probe during pooled H2/H3 checkout (#914)#915
Open
benoitc wants to merge 1 commit into
Open
Guard get_state liveness probe during pooled H2/H3 checkout (#914)#915benoitc wants to merge 1 commit into
benoitc wants to merge 1 commit into
Conversation
Reusing a pooled HTTP/2 or HTTP/3 connection probes it with hackney_conn:get_state/1, a bare gen_statem:call. If the connection is terminating at that instant (idle teardown, GOAWAY, keepalive close) the call exits, and the surrounding case only handled return values, so the exit crashed the caller of hackney:connect/4 instead of falling through to a fresh connection. Wrap both checkout probes in try/catch exit, mirroring the existing race handling in maybe_register_h2 and maybe_upgrade_ssl. A terminating connection is now treated as unusable and the checkout opens a fresh one.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #914.
Reusing a pooled HTTP/2 or HTTP/3 connection probes it with
hackney_conn:get_state/1, a baregen_statem:call. If the pooled connection is terminating at that instant (idle teardown, GOAWAY, keepalive close) the call exits. The surroundingcaseonly handled return values, so the exit propagated and crashed the caller ofhackney:connect/4({noproc,{gen_statem,call,[Pid,get_state]}}) instead of falling through to a fresh connection.The same file already guards the sibling probes for this exact race in
maybe_register_h2andmaybe_upgrade_ssl; the twoget_statecheckout probes were missed.Change
Wrap both checkout probes in
try ... catch exit:_ -> {error, terminated} end(H2 site inconnect_pool/4, H3 site intry_h3_connection/5). A terminating connection is treated as unusable and checkout falls through to the existing unregister + new-connection path. Onlyexitis caught;error/throwwould be a genuine bug and still propagate.Tests
New
hackney_connect_race_testswith ahackney_race_poolhandler that hands out an already-terminated pid on H2/H3 checkout. H2 assertshackney:connect/4returns{error, _}; H3 asserts theget_stateprobe exit does not escape. Reverting the guard makes both fail with the{noproc,...get_state...}exit. Full suite (1054 tests) passes.