Skip to content

util/async: cancellation race can block RunLoop.Append and batchRecvLoop indefinitely #2033

Description

@D3Hunter

Summary

There is a cancellation race between RunLoop.Exec(ctx) and RunLoop.Append() that can leave Append blocked forever while sending on the unbuffered ready channel.

When Append is called by async.Callback.Schedule from batchCommandsClient.batchRecvLoop, the receive loop cannot finish dispatching or retiring that response. Later responses on the same batch-command stream are then head-of-line blocked even though the gRPC connection and server are healthy.

This is reproducible on current master at 61ecd7c114166c08e747d252fb9ae592e54bb599.

Root cause

RunLoop.Append does the following:

  1. Acquire RunLoop.lock.
  2. Append the callback.
  3. If the state is StateWaiting, change it to StateIdle and set notify=true.
  4. Release the lock.
  5. Send on the unbuffered ready channel.

At the same time, RunLoop.Exec waits for either ready or ctx.Done().

The problematic interleaving is:

  1. Exec(ctx) enters StateWaiting.
  2. The context is canceled while Append is starting.
  3. Append observes StateWaiting, changes the state to StateIdle, and decides to notify Exec.
  4. Exec selects ctx.Done(), resets the state, and returns.
  5. Append executes l.ready <- struct{}{} after the only receiver has exited.
  6. The unbuffered send blocks permanently.

Callback.Schedule calls Append from inside sync.Once.Do, so the sync.Once operation also remains in progress.

For an asynchronous batch request, batchCommandsEntry.response calls Callback.Schedule. If that blocks, batchRecvLoop cannot delete/retire the request or continue processing later responses on that stream.

One observed blocked stack had this shape:

batchCommandsClient.batchRecvLoop
  -> batchCommandsEntry.response
  -> callback.Schedule
  -> sync.Once.Do
  -> RunLoop.Append
  -> chan send

Other goroutines attempting to complete the same callback can block on the sync.Once mutex. Request counters then show a growing difference between tracked and retired requests while the gRPC connections remain READY.

When EnableAsyncBatchGet is enabled, the multi-region batch-get path can reach this code through:

KVSnapshot.BatchGetWithTier
  -> batchGetKeysByRegions
  -> asyncBatchGetByRegions
  -> SendRequestAsync
  -> batchRecvLoop
  -> Callback.Schedule
  -> RunLoop.Append

Minimal reproduction

The following test uses the package lock only to force the otherwise timing-dependent interleaving. It detects the bug when Append remains blocked after the canceled Exec has returned.

Reproduction test for util/async/runloop_test.go
func TestCanceledExecCanStrandAppend(t *testing.T) {
	oldProcs := runtime.GOMAXPROCS(1)
	defer runtime.GOMAXPROCS(oldProcs)

	for attempt := 0; attempt < 100; attempt++ {
		loop := NewRunLoop()
		ctx, cancel := context.WithCancel(context.Background())
		execDone := make(chan error, 1)
		go func() {
			_, err := loop.Exec(ctx)
			execDone <- err
		}()

		deadline := time.Now().Add(time.Second)
		for loop.State() != StateWaiting {
			if time.Now().After(deadline) {
				t.Fatal("run loop did not enter waiting state")
			}
			runtime.Gosched()
		}

		// Queue Append on the mutex before waking Exec through cancellation.
		loop.lock.Lock()
		appendDone := make(chan struct{})
		go func() {
			loop.Append(func() {})
			close(appendDone)
		}()
		runtime.Gosched()
		cancel()
		runtime.Gosched()
		loop.lock.Unlock()

		select {
		case err := <-execDone:
			if err == nil {
				t.Fatal("expected canceled Exec to return an error")
			}
		case <-time.After(time.Second):
			t.Fatal("Exec did not return after cancellation")
		}

		select {
		case <-appendDone:
			// Mutex wake-up ordering may select Exec first; retry.
		case <-time.After(20 * time.Millisecond):
			t.Logf("reproduced stranded RunLoop.Append on attempt %d", attempt+1)
			return
		}
	}

	t.Fatal("did not reproduce the cancellation/notification interleaving")
}

Required imports:

import (
	"context"
	"runtime"
	"testing"
	"time"
)

Run:

go test ./util/async -run '^TestCanceledExecCanStrandAppend$' -count=20 -v

On darwin/arm64 with Go 1.25.10, all 20 runs reproduced the stranded Append on the first attempt:

reproduced stranded RunLoop.Append on attempt 1
PASS

This is a positive reproducer: it passes when the blocking bug is observed. A permanent regression test should invert the final assertion and require Append to complete.

Expected behavior

Cancellation of the RunLoop executor must not cause a concurrent or late callback submission to block indefinitely. A late response should be safely executed, rejected, or discarded without stopping the batch receive loop.

Requests following a canceled asynchronous request on the same batch-command stream should continue to receive and retire their responses.

Actual behavior

RunLoop.Append can block forever on the unbuffered wake-up send. When this happens in batchRecvLoop, one callback causes stream-wide head-of-line blocking:

  • the response being dispatched is not retired;
  • later responses on that stream are not processed;
  • tracked-but-not-retired requests accumulate;
  • unrelated client requests sharing the stream can hang or time out.

Suggested regression coverage

In addition to the util/async interleaving test, an internal/client regression test could:

  1. Start a mock batch-command server.
  2. Issue an asynchronous request using a RunLoop callback.
  3. Cancel its context concurrently with the server response.
  4. Send an unrelated synchronous request through the same batch stream.
  5. Verify that the second request completes and both request entries are retired.

Metadata

Metadata

Assignees

Labels

contributionThis PR is from a community contributor.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions