godesign-patternsconcurrencydeadlockchannel

Understanding Potential Deadlock in Resource Pool Implementation Described in "Go in Action"


I'm currently reading "Go in Action" and came across a section regarding the implementation of a resource pool in Go. The book provides a complete example, but there's a particular part about preventing deadlocks that I find confusing. Below is the implementation as described, including a comment from the book right before closing the resources channel:

package pool

import (
    "errors"
    "log"
    "io"
    "sync"
)

type Pool struct {
    m        sync.Mutex
    resources chan io.Closer
    factory   func() (io.Closer, error)
    closed    bool
}

var ErrPoolClosed = errors.New("Pool has been closed")

func New(fn func() (io.Closer, error), size uint) (*Pool, error) {
    if size <= 0 {
        return nil, errors.New("Size value too small")
    }

    return &Pool{
        factory:   fn,
        resources: make(chan io.Closer, size),
    }, nil
}

func (p *Pool) Acquire() (io.Closer, error) {
    select {
    case r, ok := <-p.resources:
        if !ok {
            return nil, ErrPoolClosed
        }
        log.Println("Acquire:", "Shared Resource")
        return r, nil
    default:
        log.Println("Acquire:", "New Resource")
        return p.factory()
    }
}

func (p *Pool) Release(r io.Closer) {
    p.m.Lock()
    defer p.m.Unlock()

    if p.closed {
        r.Close()
        return
    }

    select {
    case p.resources <- r:
        log.Println("Release:", "In Queue")
    default:
        log.Println("Release:", "Closing")
        r.Close()
    }
}

func (p *Pool) Close() {
    p.m.Lock()
    defer p.m.Unlock()

    if p.closed {
        return
    }

    p.closed = true

    // Close the channel before we drain the channel of its resources, if we don't do this, we will have a deadlock.
    close(p.resources)

    for r := range p.resources {
        r.Close()
    }
}

The comment in question is:

// Close the channel before we drain the channel of its resources, if we don't do this, we will have a deadlock.

I'm puzzled by this statement. The Acquire method employs a select statement with a default case for obtaining resources, which implies it should not block if the channel is empty; it either retrieves an existing resource or creates a new one. Given this non-blocking behavior, how does the described approach prevent a deadlock? I can't wrap my head around the deadlock scenario that the book suggests could occur if the channel isn't closed before being emptied.

Could someone help explain or clarify this potential deadlock situation? I'm looking for a deeper understanding of the concurrency mechanisms at play in this specific implementation.

What I tried:

To understand the potential deadlock scenario mentioned in "Go in Action," I carefully reviewed the implementation details of the Pool struct, especially focusing on the Close and Acquire methods. I also experimented with similar patterns in a controlled environment to observe behavior when channels are closed while being drained, versus when they are not. I was expecting to either encounter a deadlock situation myself or to understand the theoretical basis for such a deadlock from the code provided in the book.

What I was expecting:

Based on the comment in the book, I was expecting that not closing the channel before draining it would lead to a deadlock scenario. However, given the non-blocking design of the Acquire method, which uses a select statement with a default case to either fetch an available resource from the channel or create a new one, it wasn't clear how a deadlock could occur. My expectation was that the Acquire method's design would inherently prevent deadlocks by avoiding blocking on resource acquisition. Thus, I was looking for a deadlock in scenarios where, theoretically, all goroutines might end up waiting on each other to release resources, but the implementation seems to prevent this.


Solution

  • @JimB 's comment clarified the potential deadlock issue in the "Go in Action" book example about closing a channel before draining it. I missed that a for range loop on a channel hangs if the channel isn't closed, leading to the deadlock scenario the book mentioned. Big thanks to @JimB for pointing out this crucial detail!