rpcclient: resolve all batch futures if Send fails
What changed, and why it matters
This commit fixes a bug in btcd's RPC batch client. Previously, if sending a batch of RPC requests failed (for example, due to a network or server error), the client would clear its internal list of batched requests but would never signal the failure to each individual request's 'future' result object. Callers waiting for results could therefore block forever. The patch adds a helper that fans out the Send() error to every queued request and clears tracking state in one place, plus a regression test verifying queued futures complete with the same error.
Treat this as a reliability/availability fix rather than an active exploit. Users relying on batch RPC should upgrade to the patched version to avoid stuck goroutines and leaked waiters when RPC batch submissions fail. Review any long-running services using batch mode for symptoms of goroutine accumulation or stuck Receive calls.
Security signals we found
Denial-of-service via indefinite caller blocking on RPC batch failure
Resource exhaustion from goroutines waiting on unresolved futures
Missing error propagation in asynchronous batch request path
Evidence from the diff
In rpcclient/infrastructure.go, Send() previously only reset c.batchList on failure. The per-request responseChan futures were left unwritten, so Future.Receive() callers could block indefinitely. The patch introduces failBatchRequests(), which under requestLock and batchLock iterates c.batchList, sends a Response{err: err} to each req.responseChan, and resets requestMap, batchList, and requestList. A new test TestBatchSendErrorResolvesQueuedFutures simulates a non-JSON HTTP response, calls Send(), and asserts both queued GetBlockCountAsync futures resolve with the same error within a timeout.
Changed components
rpcclient/infrastructure.gorpcclient/infrastructure_test.goClient.Send()Client.failBatchRequests()batch-mode RPC futuresInspect captured patch +95 / −6
diff --git a/rpcclient/infrastructure.go b/rpcclient/infrastructure.go
index 09856c6..8018497 100644
--- a/rpcclient/infrastructure.go
+++ b/rpcclient/infrastructure.go
@@ -1762,6 +1762,38 @@ func (c *Client) sendAsync() (FutureGetBulkResult, error) {
return responseChan, nil
}
+// failBatchRequests resolves every queued batch request with the provided error
+// and clears all internal request tracking.
+//
+// This function is safe for concurrent access.
+func (c *Client) failBatchRequests(err error) {
+ c.requestLock.Lock()
+ defer c.requestLock.Unlock()
+
+ c.batchLock.Lock()
+ defer c.batchLock.Unlock()
+
+ for e := c.batchList.Front(); e != nil; e = e.Next() {
+ req := e.Value.(*jsonRequest)
+
+ // Resolve all pending futures on the first batch-level failure
+ // so callers waiting on Receive don't block indefinitely.
+ // Safe: batch-mode responseChan buffers are unwritten here,
+ // so this send won't block while locks are held. Batch-mode
+ // requests only use addRequest (not sendPostRequest), so each
+ // responseChan buffer is still empty.
+ req.responseChan <- &Response{err: err}
+ }
+
+ c.requestMap = make(map[uint64]*list.Element)
+ c.batchList = list.New()
+
+ // Batch-mode requests are tracked in batchList, so requestList should
+ // already be empty. Keep this defensive reset for invariants and future
+ // call paths.
+ c.requestList.Init()
+}
+
// Marshall's bulk requests and sends to the server
// creates a response channel to receive the response
func (c *Client) Send() error {
@@ -1772,12 +1804,7 @@ func (c *Client) Send() error {
batchResp, err := future.Receive()
if err != nil {
- // Clear batchlist in case of an error.
-
- c.batchLock.Lock()
- c.batchList = list.New()
- c.batchLock.Unlock()
-
+ c.failBatchRequests(err)
return err
}
diff --git a/rpcclient/infrastructure_test.go b/rpcclient/infrastructure_test.go
index bf09c65..5852edf 100644
--- a/rpcclient/infrastructure_test.go
+++ b/rpcclient/infrastructure_test.go
@@ -517,3 +517,65 @@ func TestSendPostRequestShutdownPrioritizesFailure(t *testing.T) {
}
}
}
+
+// TestBatchSendErrorResolvesQueuedFutures ensures a batch send failure resolves
+// all queued futures instead of leaving them blocked.
+func TestBatchSendErrorResolvesQueuedFutures(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()
+ })
+
+ client.httpClient.Transport = postRoundTripFunc(
+ func(*http.Request) (*http.Response, error) {
+ body := io.NopCloser(strings.NewReader("not-json"))
+
+ return &http.Response{
+ StatusCode: http.StatusOK,
+ Header: make(http.Header),
+ Body: body,
+ }, nil
+ },
+ )
+
+ f1 := client.GetBlockCountAsync()
+ f2 := client.GetBlockCountAsync()
+
+ sendErr := client.Send()
+ require.Error(t, sendErr)
+
+ assertFutureErr := func(f FutureGetBlockCountResult) {
+ t.Helper()
+
+ done := make(chan error, 1)
+ // Receive is the blocking caller-facing path. The old bug surfaced here
+ // by never resolving the future, so bound it with a timeout.
+ go func() {
+ _, err := f.Receive()
+ done <- err
+ }()
+
+ select {
+ case err := <-done:
+ require.Error(t, err)
+ require.EqualError(t, err, sendErr.Error())
+
+ case <-time.After(2 * time.Second):
+ t.Fatal("timed out waiting for queued batch future " +
+ "to resolve")
+ }
+ }
+
+ assertFutureErr(f1)
+ assertFutureErr(f2)
+}
Why this scored 50/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.