Skip to content

jsonrpc: serialize writes to client connection#9

Merged
benma merged 1 commit into
BitBoxSwiss:masterfrom
xoba:codex/jsonrpc-serialize-writes
May 12, 2026
Merged

jsonrpc: serialize writes to client connection#9
benma merged 1 commit into
BitBoxSwiss:masterfrom
xoba:codex/jsonrpc-serialize-writes

Conversation

@xoba
Copy link
Copy Markdown
Contributor

@xoba xoba commented Apr 30, 2026

Summary

This PR serializes writes from jsonrpc.Client to its underlying connection by adding a write mutex around the SetWriteDeadline + Write pair.

It also adds a regression test that launches concurrent MethodBlocking calls through a wrapped net.Conn which reports an error if writes overlap.

Root cause

jsonrpc.Client.Method can be called from multiple goroutines, but it previously wrote request bytes directly to the shared net.Conn without write-side synchronization. Plain TCP usually masks this because writes are internally serialized, but other net.Conn implementations can expose the overlap.

One affected transport is golang.org/x/crypto/ssh's chanConn: its Write delegates to Channel.WriteExtended, whose source currently notes that concurrent WriteExtended calls can be flagged by the race detector because a packet buffer is reused. Its SetWriteDeadline implementation also returns ssh: tcpChan: deadline not supported, so the deadline call does not protect the write path.

In downstream SSH-tunneled Electrum usage, concurrent MethodBlocking calls can corrupt the SSH packet stream, drop the connection, and cause all in-flight requests to fail together with context canceled.

Evidence

A downstream reproducer run with go run -race reported races through this call path:

  • golang.org/x/crypto/ssh.(*channel).WriteExtended
  • golang.org/x/crypto/ssh.(*channel).Write
  • golang.org/x/crypto/ssh.(*chanConn).Write
  • github.com/BitBoxSwiss/block-client-go/jsonrpc.(*Client).Method

The regression test in this PR verifies the client no longer performs overlapping writes while preserving concurrent request/response handling.

Impact

For TCP-only users, this should be behaviorally equivalent to the existing implementation, with one additional mutex acquire per request write. For SSH-tunneled or other connection implementations that are sensitive to concurrent writes, this avoids corrupting the transport and tearing down unrelated in-flight requests.

Tests

  • go test ./jsonrpc
  • go test ./...
  • go test -race ./...

Copy link
Copy Markdown
Contributor

@benma benma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@benma benma merged commit 88cd3ac into BitBoxSwiss:master May 12, 2026
2 checks passed
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