diff --git a/pool.go b/pool.go index a5f2b40..2d21c0d 100644 --- a/pool.go +++ b/pool.go @@ -21,12 +21,7 @@ const maxInt = int(maxUint >> 1) var ErrClosedPool = errors.New("cannot get from closed pool") type CreateFunc func(ctx context.Context) (res interface{}, err error) -type CloseFunc func(res interface{}) (err error) - -// BackgroundErrorHandler is the type of function that handles background -// errors. It may be called while the pool is locked. Therefore it must not call -// any pool methods and should not perform any lengthy operations. -type BackgroundErrorHandler func(err error) +type CloseFunc func(res interface{}) type resourceWrapper struct { resource interface{} @@ -47,21 +42,19 @@ type Pool struct { maxResourceCheckouts uint64 closed bool - createRes CreateFunc - closeRes CloseFunc - backgroundErrorHandler BackgroundErrorHandler + createRes CreateFunc + closeRes CloseFunc } func NewPool(createRes CreateFunc, closeRes CloseFunc) *Pool { return &Pool{ - cond: sync.NewCond(new(sync.Mutex)), - allResources: make(map[interface{}]*resourceWrapper), - maxSize: maxInt, - maxResourceDuration: math.MaxInt64, - maxResourceCheckouts: math.MaxUint64, - createRes: createRes, - closeRes: closeRes, - backgroundErrorHandler: func(error) {}, + cond: sync.NewCond(new(sync.Mutex)), + allResources: make(map[interface{}]*resourceWrapper), + maxSize: maxInt, + maxResourceDuration: math.MaxInt64, + maxResourceCheckouts: math.MaxUint64, + createRes: createRes, + closeRes: closeRes, } } @@ -72,10 +65,7 @@ func (p *Pool) Close() { p.closed = true for _, rw := range p.availableResources { - err := p.closeRes(rw.resource) - if err != nil { - p.backgroundErrorHandler(err) - } + p.closeRes(rw.resource) delete(p.allResources, rw.resource) } p.availableResources = nil @@ -166,17 +156,6 @@ func (p *Pool) SetMaxResourceCheckouts(n uint64) { p.cond.L.Unlock() } -// SetBackgroundErrorHandler assigns a handler for errors that have no other -// place to be reported. For example, Get is called when no resources are -// available. Get begins creating a new resource (in a goroutine). Before the -// new resource is completed, the context passed to Get is canceled. Then the -// new resource creation fails. f will be called with that error. -func (p *Pool) SetBackgroundErrorHandler(f BackgroundErrorHandler) { - p.cond.L.Lock() - p.backgroundErrorHandler = f - p.cond.L.Unlock() -} - // Get gets a resource from the pool. If no resources are available and the pool // is not at maximum capacity it will create a new resource. If the pool is at // maximum capacity it will block until a resource is available. ctx can be used @@ -258,17 +237,6 @@ func (p *Pool) Get(ctx context.Context) (interface{}, error) { } } -// func (p *Pool) backgroundReportError(errChan chan error) { -// go func() { -// err := <-errChan -// if err != nil { -// p.cond.L.Lock() -// p.backgroundErrorHandler(err) -// p.cond.L.Unlock() -// } -// }() -// } - // lockedAvailableGet gets the top resource from p.availableResources. p.cond.L // must already be locked. len(p.availableResources) must be > 0. func (p *Pool) lockedAvailableGet() interface{} { @@ -284,10 +252,7 @@ func (p *Pool) lockedAvailableGet() interface{} { func (p *Pool) backgroundClose(res interface{}) { go func() { - err := p.closeRes(res) - if err != nil { - p.backgroundErrorHandler(err) - } + p.closeRes(res) }() } @@ -347,11 +312,6 @@ func (p *Pool) Remove(res interface{}) { // close the resource in the background go func() { - err := p.closeRes(res) - if err != nil { - p.cond.L.Lock() - p.backgroundErrorHandler(err) - p.cond.L.Unlock() - } + p.closeRes(res) }() } diff --git a/pool_test.go b/pool_test.go index 99ed031..3bfa0f5 100644 --- a/pool_test.go +++ b/pool_test.go @@ -60,18 +60,16 @@ func createCreateResourceFuncWithNotifierChan() (puddle.CreateFunc, *Counter, ch func createCloseResourceFuncWithNotifierChan() (puddle.CloseFunc, *Counter, chan int) { ch := make(chan int) var c Counter - f := func(interface{}) error { + f := func(interface{}) { n := c.Next() // Because the tests will not read from ch until after the close function f returns. go func() { ch <- n }() - - return nil } return f, &c, ch } -func stubCloseRes(interface{}) error { return nil } +func stubCloseRes(interface{}) {} func waitForRead(ch chan int) bool { select { @@ -233,9 +231,8 @@ func TestPoolCloseClosesAllAvailableResources(t *testing.T) { createFunc, _ := createCreateResourceFunc() var closeCalls Counter - closeFunc := func(interface{}) error { + closeFunc := func(interface{}) { closeCalls.Next() - return nil } p := puddle.NewPool(createFunc, closeFunc) @@ -356,56 +353,6 @@ func TestPoolGetReturnsErrorWhenPoolIsClosed(t *testing.T) { assert.Nil(t, res) } -func TestPoolCloseResourceCloseErrorIsReported(t *testing.T) { - createFunc, _ := createCreateResourceFunc() - errCloseFailed := errors.New("close failed") - closeFunc := func(res interface{}) error { return errCloseFailed } - pool := puddle.NewPool(createFunc, closeFunc) - asyncErrChan := make(chan error, 1) - pool.SetBackgroundErrorHandler(func(err error) { asyncErrChan <- err }) - - // Get and return a resource to put something in the pool - res, err := pool.Get(context.Background()) - require.NoError(t, err) - assert.Equal(t, 1, res) - pool.Return(res) - - pool.Close() - - select { - case err = <-asyncErrChan: - assert.Equal(t, errCloseFailed, err) - default: - t.Fatal("error not reported") - } -} - -func TestPoolReturnClosesResourcePoolIsAlreadyClosedErrorIsReported(t *testing.T) { - createFunc, _ := createCreateResourceFunc() - - errCloseFailed := errors.New("close failed") - closeFunc := func(res interface{}) error { return errCloseFailed } - pool := puddle.NewPool(createFunc, closeFunc) - - asyncErrChan := make(chan error, 1) - pool.SetBackgroundErrorHandler(func(err error) { asyncErrChan <- err }) - - // Get and return a resource to put something in the pool - res, err := pool.Get(context.Background()) - require.NoError(t, err) - assert.Equal(t, 1, res) - - pool.Close() - - pool.Return(res) - select { - case err = <-asyncErrChan: - assert.Equal(t, errCloseFailed, err) - case <-time.NewTimer(time.Second).C: - t.Fatal("timed out waiting for async error") - } -} - func BenchmarkPoolGetAndReturn(b *testing.B) { benchmarks := []struct { poolSize int