Skip to content

Fix missing stream deadlines in SendRequest - #149

Open
damilolaedwards wants to merge 1 commit into
ethpandaops:masterfrom
damilolaedwards:fix/reqresp-stream-deadline
Open

Fix missing stream deadlines in SendRequest#149
damilolaedwards wants to merge 1 commit into
ethpandaops:masterfrom
damilolaedwards:fix/reqresp-stream-deadline

Conversation

@damilolaedwards

Copy link
Copy Markdown

Summary

SendRequest opened streams with a per-request timeout but never applied it to the underlying read and write operations. A peer that accepted a stream and then stopped responding could park the calling goroutine and its libp2p stream indefinitely, since the write and the response read had no deadline at all.

Sets a write deadline before sending the request and a read deadline before waiting for the response, using the existing ReqRespConfig timeout fields. This matches the pattern already used by every sibling function in the file (ReadRequest, WriteRequest, ReadResponse).

Test plan

  • Added a test that drives a real handshake against a peer that accepts the stream and never responds, and confirms the request now fails within the configured deadline instead of hanging.
  • Added a regression test confirming a normal request/response exchange still works with the deadlines in place.
  • Confirmed the new test fails against the previous code and passes with the fix.
  • go build ./..., go vet ./..., and the full package test suite all pass.

SendRequest opened streams with a per-request timeout but never applied
it to the underlying read and write operations. A peer that accepted a
stream and then stopped responding could park the calling goroutine and
its libp2p stream indefinitely, since io.ReadFull and the payload write
had no deadline at all.

Set write and read deadlines from the existing ReqRespConfig before the
write and read phases, matching the pattern already used by every
sibling function in this file (ReadRequest, WriteRequest, ReadResponse).

Adds a test that reproduces the hang against an unresponsive peer and
confirms the request now fails within the configured deadline instead
of blocking forever, plus a regression test for the normal success path.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant