What the developer wrote
Authored by Boris Nagaev
rpcclient: avoid duplicate batch POST handlers
NewBatch called New() and then called start() again. In HTTP POST mode that
created a second sendPostHandler and another shutdown-cancel goroutine, which
broke the expected single-flight serialization of POST sends.
Keep NewBatch as a semantic toggle only: rely on New() to start handlers
once, then set batch=true. A regression test now checks that batch POST
requests stay serialized through one active transport call.
What changed, and why it matters
This commit fixes a bug in btcd's RPC batch client where creating a batch client accidentally started two background goroutines for sending HTTP POST requests instead of one. That broke the intended single-file behavior: two RPC requests could be sent at the same time rather than one at a time. The patch makes NewBatch simply turn on batch mode instead of starting the handlers again, and adds a test to confirm only one POST is in flight at a time.
Upgrade to the patched version if you use rpcclient.NewBatch in HTTP POST mode, especially if your application relies on serialized RPC sends or depends on deterministic request/response ordering. No immediate emergency action is required unless concurrent POST behavior is causing operational issues.
Security signals we found
Duplicate background handler goroutines break intended single-flight serialization
Potential race between concurrent POST sends in batch mode
Regression test asserts serialized POST transport behavior
Evidence from the diff
NewBatch previously called New() (which starts sendPostHandler and a shutdown-cancel goroutine) and then called client.start() again, spawning a second sendPostHandler and shutdown-cancel goroutine. In HTTP POST mode this broke the single-flight serialization of POST sends because two goroutines were reading from sendPostChan. The patch removes the redundant start(), relying on New() to start handlers once, and only toggles client.batch = true. A regression test verifies that two queued POST requests produce maxActive == 1.
Changed components
rpcclient/infrastructure.gorpcclient/infrastructure_test.goInspect captured patch +101 / −3
diff --git a/rpcclient/infrastructure.go b/rpcclient/infrastructure.go
index 8018497..8eb3b1c 100644
--- a/rpcclient/infrastructure.go
+++ b/rpcclient/infrastructure.go
@@ -1610,13 +1610,18 @@ func NewBatch(config *ConnConfig) (*Client, error) {
if !config.HTTPPostMode {
return nil, errors.New("http post mode is required to use batch client")
}
- // notification parameter is nil since notifications are not supported in POST mode.
+
+ // The notification parameter is nil since notifications are not
+ // supported in POST mode.
client, err := New(config, nil)
if err != nil {
return nil, err
}
- client.batch = true //copy the client with changed batch setting
- client.start()
+
+ // New() already started the HTTP handlers, so only toggle batch
+ // semantics.
+ client.batch = true
+
return client, nil
}
diff --git a/rpcclient/infrastructure_test.go b/rpcclient/infrastructure_test.go
index 5852edf..98ca117 100644
--- a/rpcclient/infrastructure_test.go
+++ b/rpcclient/infrastructure_test.go
@@ -579,3 +579,96 @@ func TestBatchSendErrorResolvesQueuedFutures(t *testing.T) {
assertFutureErr(f1)
assertFutureErr(f2)
}
+
+// TestNewBatchSerializesPostSends ensures a batch client still serializes POST
+// sends through a single handler goroutine.
+func TestNewBatchSerializesPostSends(t *testing.T) {
+ connCfg := &ConnConfig{
+ Host: "127.0.0.1:8332",
+ User: "user",
+ Pass: "pass",
+ DisableTLS: true,
+ HTTPPostMode: true,
+ }
+
+ client, err := NewBatch(connCfg)
+ require.NoError(t, err)
+ t.Cleanup(func() {
+ client.Shutdown()
+ client.WaitForShutdown()
+ })
+
+ var active int32
+ var maxActive int32
+ release := make(chan struct{})
+
+ client.httpClient.Transport = postRoundTripFunc(
+ func(*http.Request) (*http.Response, error) {
+ current := atomic.AddInt32(&active, 1)
+ for {
+ prev := atomic.LoadInt32(&maxActive)
+ if current <= prev {
+ break
+ }
+ if atomic.CompareAndSwapInt32(
+ &maxActive, prev, current,
+ ) {
+ break
+ }
+ }
+
+ // Hold the request open so the test can observe if
+ // a second POST enters the transport concurrently.
+ <-release
+ atomic.AddInt32(&active, -1)
+
+ return &http.Response{
+ StatusCode: http.StatusOK,
+ Header: make(http.Header),
+ Body: io.NopCloser(strings.NewReader(
+ `{"result":1,"error":null}`,
+ )),
+ }, nil
+ },
+ )
+
+ makeReq := func(id uint64) *jsonRequest {
+ return &jsonRequest{
+ id: id,
+ method: "getblockcount",
+ marshalledJSON: []byte(
+ `{"jsonrpc":"1.0","id":1,` +
+ `"method":"getblockcount","params":[]}`,
+ ),
+ responseChan: make(chan *Response, 1),
+ }
+ }
+
+ req1 := makeReq(1)
+ req2 := makeReq(2)
+ client.sendPostChan <- req1
+ client.sendPostChan <- req2
+
+ // Wait until one request is definitely in flight before checking whether
+ // a duplicate handler can start a second concurrent POST.
+ require.Eventually(t, func() bool {
+ return atomic.LoadInt32(&active) >= 1
+ }, time.Second, 5*time.Millisecond)
+
+ // Allow any extra send handler goroutines to start a second in-flight
+ // request.
+ time.Sleep(100 * time.Millisecond)
+ observedMax := atomic.LoadInt32(&maxActive)
+ close(release)
+
+ for i, req := range []*jsonRequest{req1, req2} {
+ select {
+ case resp := <-req.responseChan:
+ require.NoError(t, resp.err, "request %d failed", i)
+ case <-time.After(2 * time.Second):
+ t.Fatalf("timed out waiting for request %d response", i)
+ }
+ }
+
+ require.EqualValues(t, 1, observedMax, "POST sends must be serialized")
+}
Why this scored 44/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.