Skip to content

[SharovBot] rpc: fix deadlock in client dispatch when connection lost during write#20898

Open
erigon-copilot[bot] wants to merge 2 commits intomainfrom
fix-websocket-large-call-deadlock
Open

[SharovBot] rpc: fix deadlock in client dispatch when connection lost during write#20898
erigon-copilot[bot] wants to merge 2 commits intomainfrom
fix-websocket-large-call-deadlock

Conversation

@erigon-copilot
Copy link
Copy Markdown
Contributor

Summary

  • Fix deadlock in Client.dispatch when a WebSocket connection is lost while a write is in-flight
  • When the server rejects an oversized message (exceeding SetReadLimit), cancelAllRequests excludes the in-flight request (lastOp) to allow reconnection. If the write then succeeds without reconnecting, no reader exists to deliver a response, orphaning the request and causing Call to block forever.
  • When reqSent completes with no error but reading == false, the orphaned request is now immediately failed with errDead

Test plan

  • TestWebsocketLargeCall passes 10 consecutive times with -race -count=1 -timeout 2m
  • Full ./rpc/... package tests pass with -race -count=1 -timeout 5m
  • go build ./... succeeds
  • No test files modified

🤖 Generated with Claude Code

When a WebSocket server rejects an oversized message (exceeding SetReadLimit),
the connection is closed. If the client's read loop detects this closure while
a write is still in-flight, cancelAllRequests intentionally excludes the
in-flight request (lastOp) to allow a potential reconnection to re-register it.

However, if the write completes successfully without triggering a reconnect,
no reader exists to deliver a response. The excluded request's resp channel
is never closed, causing client.Call to block indefinitely.

Fix: when reqSent fires with a successful write but reading is false (no active
reader), fail the orphaned request immediately with errDead so the caller
doesn't hang.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-authored-by: Giulio Rebuffo <giulio.rebuffo@gmail.com>
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.

2 participants