fix(pool): prevent double freeTurn in queuedNewConn

This commit fixes a critical race condition where freeTurn() could be
called twice in the connection pool's queuedNewConn flow, causing turn
counter inconsistency.

Problem:
- When a new connection creation failed in queuedNewConn, both the
  defer handler and the dialing goroutine could call freeTurn()
- This led to turn counter underflow and queue length inconsistency

Solution:
- Modified putIdleConn to return a boolean indicating whether the
  caller needs to call freeTurn()
  - Returns true: connection was put back to pool, caller must free turn
  - Returns false: connection was delivered to a waiting request,
    turn will be freed by the receiving goroutine
- Updated queuedNewConn to only call freeTurn() when putIdleConn
  returns true
- Improved error handling flow in the dialing goroutine

Changes:
- putIdleConn now returns bool instead of void
- Added comprehensive documentation for putIdleConn behavior
- Refactored error handling in queuedNewConn goroutine
- Updated test cases to reflect correct turn state expectations

This ensures each turn is freed exactly once, preventing resource
leaks and maintaining correct queue state.

Author: cyningsun

internal/pool/pool.go

@@ -568,8 +568,7 @@ func (p *ConnPool) queuedNewConn(ctx context.Context) (*Conn, error) {
 	var err error
 	defer func() {
 		if err != nil {
-			if cn := w.cancel(); cn != nil {
-				p.putIdleConn(ctx, cn)
+			if cn := w.cancel(); cn != nil && p.putIdleConn(ctx, cn) {
 				p.freeTurn()
 			}
 		}
@@ -593,14 +592,15 @@ func (p *ConnPool) queuedNewConn(ctx context.Context) (*Conn, error) {
 
 		dialCtx := w.getCtxForDial()
 		cn, cnErr := p.newConn(dialCtx, true)
-		delivered := w.tryDeliver(cn, cnErr)
-		if cnErr == nil && delivered {
-			return
-		} else if cnErr == nil && !delivered {
-			p.putIdleConn(dialCtx, cn)
+		if cnErr != nil {
+			w.tryDeliver(nil, cnErr) // deliver error to caller, notify connection creation failed
 			p.freeTurn()
 			freeTurnCalled = true
-		} else {
+			return
+		}
+
+		delivered := w.tryDeliver(cn, cnErr)
+		if !delivered && p.putIdleConn(dialCtx, cn) {
 			p.freeTurn()
 			freeTurnCalled = true
 		}
@@ -616,14 +616,20 @@ func (p *ConnPool) queuedNewConn(ctx context.Context) (*Conn, error) {
 	}
 }
 
-func (p *ConnPool) putIdleConn(ctx context.Context, cn *Conn) {
+// putIdleConn puts a connection back to the pool or passes it to the next waiting request.
+//
+// It returns true if the connection was put back to the pool,
+// which means the turn needs to be freed directly by the caller,
+// or false if the connection was passed to the next waiting request,
+// which means the turn will be freed by the waiting goroutine after it returns.
+func (p *ConnPool) putIdleConn(ctx context.Context, cn *Conn) bool {
 	for {
 		w, ok := p.dialsQueue.dequeue()
 		if !ok {
 			break
 		}
 		if w.tryDeliver(cn, nil) {
-			return
+			return false
 		}
 	}
 
@@ -632,12 +638,14 @@ func (p *ConnPool) putIdleConn(ctx context.Context, cn *Conn) {
 
 	if p.closed() {
 		_ = cn.Close()
-		return
+		return true
 	}
 
 	// poolSize is increased in newConn
 	p.idleConns = append(p.idleConns, cn)
 	p.idleConnsLen.Add(1)
+
+	return true
 }
 
 func (p *ConnPool) waitTurn(ctx context.Context) error {

internal/pool/pool_test.go

@@ -1020,22 +1020,22 @@ var _ = Describe("queuedNewConn", func() {
 		Expect(reqBErr).NotTo(HaveOccurred(), "Request B should receive Request A's connection")
 		Expect(reqBConn).NotTo(BeNil())
 
-		// CRITICAL CHECK: Turn leak detection
+		// FIRST CRITICAL CHECK: Turn state after connection delivery
 		// After Request B receives connection from putIdleConn:
-		// - Request A's turn SHOULD be released (via freeTurn)
-		// - Request B's turn is still held (will release on Put)
-		// Expected QueueLen: 1 (only Request B)
-		// If Bug exists (missing freeTurn): QueueLen: 2 (Request A's turn leaked)
-		time.Sleep(100 * time.Millisecond) // Allow time for turn release
-		currentQueueLen := testPool.QueueLen()
-
-		Expect(currentQueueLen).To(Equal(1),
-			"QueueLen should be 1 (only Request B holding turn). "+
-				"If it's 2, Request A's turn leaked due to missing freeTurn()")
-
-		// Cleanup
+		// - Request A's turn is held by Request B (connection delivered)
+		// - Request B's turn is still held by Request B's dial to complete the connection
+		// Expected QueueLen: 2 (Request B holding turn for connection usage)
+		time.Sleep(100 * time.Millisecond) // ~300ms total
+		Expect(testPool.QueueLen()).To(Equal(2))
+
+		// SECOND CRITICAL CHECK: Turn release after dial completion
+		// Wait for Request B's dial result to complete
+		time.Sleep(300 * time.Millisecond) // ~600ms total
+		Expect(testPool.QueueLen()).To(Equal(1))
+
+		// Cleanup and verify turn is released
 		testPool.Put(ctx, reqBConn)
-		Eventually(func() int { return testPool.QueueLen() }, "500ms").Should(Equal(0))
+		Eventually(func() int { return testPool.QueueLen() }, "600ms").Should(Equal(0))
 	})
 })