Skip to content

Commit

Permalink
Fix resource pool release / discard to not waste cpu on generating be…
Browse files Browse the repository at this point in the history
…nign error.
  • Loading branch information
Patrick Lee committed Oct 16, 2014
1 parent 8ded1bb commit 62ac705
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 15 deletions.
20 changes: 14 additions & 6 deletions resource_pool/managed_handle.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ type ManagedHandle interface {
// This returns the resource pool which owns this handle.
Owner() ResourcePool

// The releases the underlying resource handle to the caller and marks the
// managed handle as inactive. The caller is responsible for cleaning up
// the released handle. This returns nil if the managed handle no longer
// owns the resource.
ReleaseUnderlyingHandle() interface{}

// This indictes a user is done with the handle and releases the handle
// back to the resource pool.
Release() error
Expand Down Expand Up @@ -73,17 +79,19 @@ func (c *ManagedHandleImpl) Owner() ResourcePool {
}

// See ManagedHandle for documentation.
func (c *ManagedHandleImpl) Release() error {
func (c *ManagedHandleImpl) ReleaseUnderlyingHandle() interface{} {
if atomic.CompareAndSwapInt32(&c.isActive, 1, 0) {
return c.pool.Release(c)
return c.handle
}
return nil
}

// See ManagedHandle for documentation.
func (c *ManagedHandleImpl) Release() error {
return c.pool.Release(c)
}

// See ManagedHandle for documentation.
func (c *ManagedHandleImpl) Discard() error {
if atomic.CompareAndSwapInt32(&c.isActive, 1, 0) {
return c.pool.Discard(c)
}
return nil
return c.pool.Discard(c)
}
20 changes: 11 additions & 9 deletions resource_pool/simple_resource_pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,11 @@ func (p *SimpleResourcePool) Release(handle ManagedHandle) error {
"by another resource pool")
}

atomic.AddInt32(p.numActive, -1)

h, _ := handle.Handle()
p.queueIdleHandles(h)
h := handle.ReleaseUnderlyingHandle()
if h != nil {
atomic.AddInt32(p.numActive, -1)
p.queueIdleHandles(h)
}

return nil
}
Expand All @@ -159,11 +160,12 @@ func (p *SimpleResourcePool) Discard(handle ManagedHandle) error {
"by another resource pool")
}

atomic.AddInt32(p.numActive, -1)

h, _ := handle.Handle()
if err := p.options.Close(h); err != nil {
return errors.Wrap(err, "Failed to close resource handle")
h := handle.ReleaseUnderlyingHandle()
if h != nil {
atomic.AddInt32(p.numActive, -1)
if err := p.options.Close(h); err != nil {
return errors.Wrap(err, "Failed to close resource handle")
}
}
return nil
}
Expand Down
54 changes: 54 additions & 0 deletions resource_pool/simple_resource_pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,60 @@ func (s *SimpleResourcePoolSuite) TestRecycleHandles(c *C) {
CheckId(c, n4, 5)
}

func (s *SimpleResourcePoolSuite) TestDoubleFree(c *C) {
dialer := fakeDialer{}
mockClock := time2.MockClock{}

options := Options{
MaxIdleHandles: 10,
Open: dialer.FakeDial,
Close: closeMockConn,
NowFunc: mockClock.Now,
}

pool := NewSimpleResourcePool(options).(*SimpleResourcePool)
pool.Register("bar")

c1, err := pool.Get("bar")
c.Assert(err, IsNil)

c2, err := pool.Get("bar")
c.Assert(err, IsNil)

c.Assert(dialer.MaxId(), Equals, 2)
c.Assert(pool.NumActive(), Equals, int32(2))
c.Assert(pool.NumIdle(), Equals, 0)

err = c1.Release()
c.Assert(err, IsNil)

err = c1.Release()
c.Assert(err, IsNil)

err = c1.Discard()
c.Assert(err, IsNil)

c.Assert(dialer.MaxId(), Equals, 2)
c.Assert(pool.NumActive(), Equals, int32(1))
c.Assert(pool.NumIdle(), Equals, 1)

err = c2.Discard()
c.Assert(err, IsNil)

err = c2.Discard()
c.Assert(err, IsNil)

err = c2.Release()
c.Assert(err, IsNil)

c.Assert(dialer.MaxId(), Equals, 2)
c.Assert(pool.NumActive(), Equals, int32(0))
c.Assert(pool.NumIdle(), Equals, 1)

c.Assert(c1.ReleaseUnderlyingHandle(), IsNil)
c.Assert(c2.ReleaseUnderlyingHandle(), IsNil)
}

func (s *SimpleResourcePoolSuite) TestDiscards(c *C) {
dialer := fakeDialer{}
mockClock := time2.MockClock{}
Expand Down

0 comments on commit 62ac705

Please sign in to comment.