The Problem: A Silent Goroutine Leak

A community contributor recently identified a resource leak in a Go worker pool implementation. The issue centered on the dynamic scaling mechanism that failed to terminate its monitoring goroutine when the pool was shut down.

The problematic function:

// scaleWorkers dynamically adjusts the pool size based on queue depth
func (p *workerPool) scaleWorkers() {
    scalingTicker := time.NewTicker(p.scalingInterval)
    defer scalingTicker.Stop()

    for range scalingTicker.C {
        p.mutex.Lock()
        queueDepth := len(p.jobQueue)
        currentSize := len(p.idleWorkers)
        
        if queueDepth > currentSize*3/4 && currentSize < p.maxSize {
            // Scale up: double workers until reaching max
            newCount := min(currentSize*2, p.maxSize) - currentSize
            for i := 0; i < newCount; i++ {
                w := newWorker()
                p.workers = append(p.workers, w)
                p.idleWorkers = append(p.idleWorkers, len(p.workers)-1)
                w.start(p, len(p.workers)-1)
            }
        } else if queueDepth == 0 && currentSize > p.minSize {
            // Scale down: halve workers until reaching min
            removeCount := max((currentSize-p.minSize)/2, p.minSize)
            p.workers = p.workers[:len(p.workers)-removeCount]
            p.idleWorkers = p.idleWorkers[:len(p.idleWorkers)-removeCount]
        }
        p.mutex.Unlock()
    }
}

This grooutine would run indefinitely, even after calling Release(), creating a subtle goroutine leak.

Initial PR Review with GPT-4

Instead of manually parsing the diff, the changes were exported and analyzed using GPT-4. The contributor's fix introduced a shutdown channel to signal termination:

type workerPool struct {
    // ... existing fields
    shutdownChan chan struct{}
}

func (p *workerPool) scaleWorkers() {
    scalingTicker := time.NewTicker(p.scalingInterval)
    defer scalingTicker.Stop()

    for {
        select {
        case <-scalingTicker.C:
            // ... scaling logic remains unchanged
        case <-p.shutdownChan:
            return
        }
    }
}

func (p *workerPool) Release() {
    close(p.jobQueue)
    close(p.shutdownChan)
    // ... wait for workers
}

GPT-4 confirmed the approach was functionally correct. The channel-based termination pattern provides explicit control but couples the scaling logic directly to the pool's lifecycle.

Refactoring to Context-Based Cancellation

While the channel approach works, Go's context package offers more idiomatic goroutien lifecycle management. GPT-4 was tasked with refactoring the implementation.

The refactored design replaces the shutdown channel with a cancellable context:

package pool

import (
    "context"
    "sync"
    "time"
)

type workerPool struct {
    shutdownCtx    context.Context
    shutdownCancel context.CancelFunc
    scalingInterval time.Duration
    jobQueue       chan Task
    idleWorkers    []int
    workers        []*worker
    mutex          sync.Mutex
    minSize        int
    maxSize        int
}

func NewWorkerPool(maxSize int, opts ...Option) *workerPool {
    shutdownCtx, shutdownCancel := context.WithCancel(context.Background())
    pool := &workerPool{
        shutdownCtx:    shutdownCtx,
        shutdownCancel: shutdownCancel,
        maxSize:        maxSize,
        minSize:        1, // default
        scalingInterval: time.Second * 5,
        jobQueue:       make(chan Task, maxSize*2),
        idleWorkers:    make([]int, 0, maxSize),
        workers:        make([]*worker, 0, maxSize),
    }
    
    // Apply options and start initial workers
    for _, opt := range opts {
        opt(pool)
    }
    
    pool.initializeWorkers()
    go pool.scaleWorkers()
    
    return pool
}

func (p *workerPool) Release() {
    p.shutdownCancel() // Signal all goroutines to exit
    close(p.jobQueue)
    
    // Wait for workers to finish
    p.wg.Wait()
}

func (p *workerPool) scaleWorkers() {
    scalingTicker := time.NewTicker(p.scalingInterval)
    defer scalingTicker.Stop()

    for {
        select {
        case <-scalingTicker.C:
            p.mutex.Lock()
            queueDepth := len(p.jobQueue)
            currentSize := len(p.idleWorkers)

            if queueDepth > currentSize*3/4 && currentSize < p.maxSize {
                newCount := min(currentSize*2, p.maxSize) - currentSize
                for i := 0; i < newCount; i++ {
                    w := newWorker()
                    p.workers = append(p.workers, w)
                    p.idleWorkers = append(p.idleWorkers, len(p.workers)-1)
                    w.start(p, len(p.workers)-1)
                }
            } else if queueDepth == 0 && currentSize > p.minSize {
                removeCount := max((currentSize-p.minSize)/2, p.minSize)
                p.workers = p.workers[:len(p.workers)-removeCount]
                p.idleWorkers = p.idleWorkers[:len(p.idleWorkers)-removeCount]
            }
            p.mutex.Unlock()

        case <-p.shutdownCtx.Done():
            return
        }
    }
}

Key Improvements

The context-based approach provides several advantages:

• Hierarchy-aware: Contexts can propagate cancellation across multiple goroutine hierarchies
• Deadline support: Future enhancements could include timeout-based shutdown using context.WithTimeout
• Standard pattern: Aligns with Go best practices for managing long-running operations
• Single signal source: One cancellation call terminates all context-aware goroutines

The refactored code passed all tests and was merged as a follow-up improvement, demonstrating how AI-assisted review can both validate fixes and suggest more idiomatic solutions.