Skip to content

Fix missing write deadline in writeRLPx - #150

Open
damilolaedwards wants to merge 1 commit into
ethpandaops:masterfrom
damilolaedwards:fix/execution-write-deadline
Open

Fix missing write deadline in writeRLPx#150
damilolaedwards wants to merge 1 commit into
ethpandaops:masterfrom
damilolaedwards:fix/execution-write-deadline

Conversation

@damilolaedwards

Copy link
Copy Markdown

Summary

writeRLPx locked writeMu and wrote to the rlpx connection with no write deadline. A peer that completed the handshake and then stopped reading could stall the write indefinitely while the lock was held, wedging the session and every other sender behind it, since they all serialize on the same mutex.

Sets a write deadline before the write, mirroring the read deadline already applied each iteration of the session loop. The timeout is a package variable rather than a constant so tests can shrink it instead of waiting out the full duration.

Test plan

  • Added a test that drives a real handshake against an unresponsive peer and confirms the write now fails within the deadline instead of blocking forever.
  • Verified the test fails against the previous code (a goroutine-local reference to the accepted connection let the GC finalizer close the socket early during an earlier draft, producing a misleading result; fixed by having the connection owned by a long-lived test peer struct instead) and passes with the fix.
  • go build ./..., go vet ./..., and the full package test suite all pass.

writeRLPx locked writeMu and wrote to the rlpx connection with no write
deadline. A peer that completed the handshake and then stopped reading
could stall the write indefinitely while the lock was held, wedging the
session and every other sender behind it, since they all serialize on
the same mutex.

Set a write deadline before the write, mirroring the read deadline
already applied each iteration of the session loop. The timeout is a
package variable rather than a constant so tests can shrink it instead
of waiting out the full duration.

Adds a test that drives a real handshake against an unresponsive peer
and confirms the write now fails within the deadline instead of
blocking forever.
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