diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index c2d24ec925..8a9a52f319 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -14,6 +14,8 @@ jobs: benchmark: name: benchmark runs-on: ubuntu-latest + # Run benchmarks only for pushes or same-repo PRs; skip for fork PRs + if: ${{ github.event_name == 'push' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository) }} strategy: fail-fast: false matrix: @@ -26,8 +28,7 @@ jobs: - "8.0.x" # Redis CE 8.0 go-version: - "1.25.x" - - oldstable - - stable + - "1.24.x" steps: - name: Set up ${{ matrix.go-version }} @@ -77,6 +78,8 @@ jobs: test-redis-ce: name: test-redis-ce runs-on: ubuntu-latest + # Run full CE matrix only for pushes or same-repo PRs; skip on fork PRs + if: ${{ github.event_name == 'push' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository) }} strategy: fail-fast: false matrix: @@ -89,8 +92,7 @@ jobs: - "8.0.x" # Redis CE 8.0 go-version: - "1.25.x" - - oldstable - - stable + - "1.24.x" steps: - name: Checkout code @@ -107,4 +109,3 @@ jobs: with: files: coverage.txt token: ${{ secrets.CODECOV_TOKEN }} - diff --git a/.github/workflows/govulncheck.yml b/.github/workflows/govulncheck.yml index fdbd056d40..bca143f16a 100644 --- a/.github/workflows/govulncheck.yml +++ b/.github/workflows/govulncheck.yml @@ -21,6 +21,8 @@ jobs: name: govulncheck runs-on: ubuntu-latest timeout-minutes: 15 + # Skip on fork PRs; run for pushes or same-repo PRs + if: ${{ github.event_name == 'push' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository) }} steps: - name: Checkout code @@ -29,7 +31,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v7 with: - go-version: "1.26.x" + go-version: "1.24.x" cache: true - name: Install govulncheck diff --git a/.github/workflows/test-e2e.yml b/.github/workflows/test-e2e.yml index f53d878717..9302bfd2dd 100644 --- a/.github/workflows/test-e2e.yml +++ b/.github/workflows/test-e2e.yml @@ -13,11 +13,14 @@ jobs: test-e2e-mock: name: E2E Tests (Mock Proxy) runs-on: ubuntu-latest + # Run E2E only for pushes or same-repo PRs; skip on fork PRs + if: ${{ github.event_name == 'push' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository) }} strategy: fail-fast: false matrix: go-version: - - stable + - "1.25.x" + - "1.24.x" steps: - name: Checkout code @@ -59,4 +62,3 @@ jobs: docker logs cae-resp-proxy 2>&1 | tail -100 echo "=== proxy-fault-injector logs ===" docker logs proxy-fault-injector 2>&1 | tail -100 - diff --git a/brpop_ctx_cancel_test.go b/brpop_ctx_cancel_test.go new file mode 100644 index 0000000000..8a34c398a4 --- /dev/null +++ b/brpop_ctx_cancel_test.go @@ -0,0 +1,45 @@ +package redis_test + +import ( + "context" + "testing" + "time" + + "github.com/redis/go-redis/v9" +) + +// TestBRPopContextCancellation verifies that a blocking BRPop with infinite timeout +// respects ctx.Done() and returns promptly with context.Canceled. +func TestBRPopContextCancellation(t *testing.T) { + opt := redis.Options{ + Addr: ":6379", + ReadTimeout: -1, // block indefinitely for reads + WriteTimeout: -1, + ContextTimeoutEnabled: true, + } + rdb := redis.NewClient(&opt) + t.Cleanup(func() { _ = rdb.Close() }) + + key := "brpop-cancel-key" + _ = rdb.Del(context.Background(), key).Err() + + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan error, 1) + go func() { + _, err := rdb.BRPop(ctx, 0, key).Result() + done <- err + }() + + // Ensure BRPop is blocked + time.Sleep(50 * time.Millisecond) + cancel() + + select { + case err := <-done: + if err == nil || err != context.Canceled { + t.Fatalf("expected context.Canceled, got %v", err) + } + case <-time.After(2 * time.Second): + t.Fatal("BRPop did not return after context cancellation") + } +} diff --git a/internal/pool/conn.go b/internal/pool/conn.go index 36a673c650..393e3949ac 100644 --- a/internal/pool/conn.go +++ b/internal/pool/conn.go @@ -91,7 +91,7 @@ type Conn struct { // State machine for connection state management // Replaces: usable, Inited, used // Provides thread-safe state transitions with FIFO waiting queue - // States: CREATED → INITIALIZING → IDLE ⇄ IN_USE + // States: CREATED → INITIALIZING → IDLE ↔ IN_USE // ↓ // UNUSABLE (handoff/reauth) // ↓ @@ -320,7 +320,7 @@ func (cn *Conn) IsInited() bool { // This is the preferred method for acquiring a connection from the pool, as it // ensures that only one goroutine marks the connection as used. // -// Implementation: Uses state machine transitions IDLE ⇄ IN_USE +// Implementation: Uses state machine transitions IDLE ↔ IN_USE // // Returns true if the swap was successful (old value matched), false otherwise. // Deprecated: Use GetStateMachine().TryTransition() directly for better state management. @@ -907,6 +907,14 @@ func (cn *Conn) RemoteAddr() net.Addr { func (cn *Conn) WithReader( ctx context.Context, timeout time.Duration, fn func(rd *proto.Reader) error, ) error { + // Fast cancellation path: if the context is done, abort before any socket ops. + if ctx != nil { + if err := ctx.Err(); err != nil { + return err + } + } + + var cancelWatchDone chan struct{} if timeout >= 0 { // Use relaxed timeout if set, otherwise use provided timeout effectiveTimeout := cn.getEffectiveReadTimeout(timeout) @@ -917,11 +925,33 @@ func (cn *Conn) WithReader( return errConnectionNotAvailable } - if err := netConn.SetReadDeadline(cn.deadline(ctx, effectiveTimeout)); err != nil { + // Compute and set initial read deadline + dl := cn.deadline(ctx, effectiveTimeout) + if err := netConn.SetReadDeadline(dl); err != nil { return err } + + // If we have no read deadline (e.g., BRPop(timeout=0) and no ctx deadline) + // but we do have a context, spawn a watcher to force an immediate deadline + // when ctx.Done() fires. This unblocks an in-flight Read without closing + // the socket and without affecting the common hot path where a deadline exists. + if ctx != nil && dl.Equal(noDeadline) { + cancelWatchDone = make(chan struct{}) + go func(nc net.Conn, done <-chan struct{}, c context.Context) { + select { + case <-c.Done(): + _ = nc.SetReadDeadline(time.Unix(0, getCachedTimeNs())) + case <-done: + } + }(netConn, cancelWatchDone, ctx) + } } - return fn(cn.rd) + + err := fn(cn.rd) + if cancelWatchDone != nil { + close(cancelWatchDone) + } + return err } func (cn *Conn) WithWriter( @@ -1005,6 +1035,10 @@ func (cn *Conn) deadline(ctx context.Context, timeout time.Duration) time.Time { } if ctx != nil { + // If context is already done, force immediate deadline to unblock socket ops. + if err := ctx.Err(); err != nil { + return time.Unix(0, nowNs) + } deadline, ok := ctx.Deadline() if ok { if timeout == 0 {