fix go test race conditation

Author: jzwlqx

pkg/pool/pool.go

@@ -173,25 +173,37 @@ func (p *simpleObjectPool) dispose(res types.NetworkResource) {
 	}
 }
 
-func (p *simpleObjectPool) tooManyIdle() bool {
-	return p.idle.Size() > p.maxIdle || (p.idle.Size() > 0 && p.size() > p.capacity)
+func (p *simpleObjectPool) tooManyIdleLocked() bool {
+	return p.idle.Size() > p.maxIdle || (p.idle.Size() > 0 && p.sizeLocked() > p.capacity)
+}
+
+func (p *simpleObjectPool) peekOverfullIdle() *poolItem {
+	p.lock.Lock()
+	defer p.lock.Unlock()
+
+	if !p.tooManyIdleLocked() {
+		return nil
+	}
+
+	item := p.idle.Peek()
+	if item == nil {
+		return nil
+	}
+
+	if item.reverse.After(time.Now()) {
+		return nil
+	}
+	return p.idle.Pop()
 }
 
 //found resources that can be disposed, put them into dispose channel
-//must in lock
 func (p *simpleObjectPool) checkIdle() {
-	for p.tooManyIdle() {
-		p.lock.Lock()
-		item := p.idle.Peek()
+	for {
+		item := p.peekOverfullIdle()
 		if item == nil {
-			//impossible
-			break
-		}
-		if item.reverse.After(time.Now()) {
 			break
 		}
-		item = p.idle.Pop()
-		p.lock.Unlock()
+
 		res := item.res
 		log.Infof("try dispose res %+v", res)
 		err := p.factory.Dispose(res)
@@ -201,36 +213,39 @@ func (p *simpleObjectPool) checkIdle() {
 			log.Warnf("error dispose res: %+v", err)
 			p.AddIdle(res)
 		}
+
 	}
 }
 
 func (p *simpleObjectPool) preload() error {
+	p.lock.Lock()
+	defer p.lock.Unlock()
 	for {
 		// init resource sequential to avoid huge creating request on startup
 		if p.idle.Size() >= p.minIdle {
 			break
 		}
 
-		if p.size() >= p.capacity {
+		if p.sizeLocked() >= p.capacity {
 			break
 		}
 
 		res, err := p.factory.Create()
 		if err != nil {
 			return err
 		}
-		p.AddIdle(res)
+		p.idle.Push(&poolItem{res: res, reverse: time.Now()})
 	}
 
-	tokenCount := p.capacity - p.size()
+	tokenCount := p.capacity - p.sizeLocked()
 	for i := 0; i < tokenCount; i++ {
 		p.tokenCh <- struct{}{}
 	}
 
 	return nil
 }
 
-func (p *simpleObjectPool) size() int {
+func (p *simpleObjectPool) sizeLocked() int {
 	return p.idle.Size() + len(p.inuse)
 }
 
@@ -254,7 +269,7 @@ func (p *simpleObjectPool) Acquire(ctx context.Context, resID string) (types.Net
 		log.Infof("acquire (expect %s): return idle %s", resID, res.GetResourceID())
 		return res, nil
 	}
-	size := p.size()
+	size := p.sizeLocked()
 	if size >= p.capacity {
 		p.lock.Unlock()
 		log.Infof("acquire (expect %s), size %d, capacity %d: return err %v", resID, size, p.capacity, ErrNoAvailableResource)

pkg/pool/pool_test.go

@@ -11,13 +11,13 @@ import (
 )
 
 type mockObjectFactory struct {
-	createDelay    time.Duration
-	disposeDeplay  time.Duration
-	err            error
-	totalCreated   int
-	totalDisplosed int
-	idGenerator    int
-	lock           sync.Mutex
+	createDelay   time.Duration
+	disposeDeplay time.Duration
+	err           error
+	totalCreated  int
+	totalDisposed int
+	idGenerator   int
+	lock          sync.Mutex
 }
 
 type mockNetworkResource struct {
@@ -55,24 +55,36 @@ func (f *mockObjectFactory) Dispose(types.NetworkResource) error {
 	time.Sleep(f.disposeDeplay)
 	f.lock.Lock()
 	defer f.lock.Unlock()
-	f.totalDisplosed++
+	f.totalDisposed++
 	return f.err
 }
 
+func (f *mockObjectFactory) getTotalDisposed() int {
+	f.lock.Lock()
+	defer f.lock.Unlock()
+	return f.totalDisposed
+}
+
+func (f *mockObjectFactory) getTotalCreated() int {
+	f.lock.Lock()
+	defer f.lock.Unlock()
+	return f.totalCreated
+}
+
 func TestInitializerWithoutAutoCreate(t *testing.T) {
 	factory := &mockObjectFactory{}
 	createPool(factory, 3, 0)
 	time.Sleep(time.Second)
-	assert.Equal(t, 0, factory.totalCreated)
-	assert.Equal(t, 0, factory.totalDisplosed)
+	assert.Equal(t, 0, factory.getTotalCreated())
+	assert.Equal(t, 0, factory.getTotalDisposed())
 }
 
 func TestInitializerWithAutoCreate(t *testing.T) {
 	factory := &mockObjectFactory{}
 	createPool(factory, 0, 0)
 	time.Sleep(time.Second)
-	assert.Equal(t, 3, factory.totalCreated)
-	assert.Equal(t, 0, factory.totalDisplosed)
+	assert.Equal(t, 3, factory.getTotalCreated())
+	assert.Equal(t, 0, factory.getTotalDisposed())
 }
 
 func createPool(factory ObjectFactory, initIdle, initInuse int) ObjectPool {
@@ -105,39 +117,39 @@ func TestInitializerExceedMaxIdle(t *testing.T) {
 	factory := &mockObjectFactory{}
 	createPool(factory, 6, 0)
 	time.Sleep(1 * time.Second)
-	assert.Equal(t, 0, factory.totalCreated)
-	assert.Equal(t, 1, factory.totalDisplosed)
+	assert.Equal(t, 0, factory.getTotalCreated())
+	assert.Equal(t, 1, factory.getTotalDisposed())
 }
 
 func TestInitializerExceedCapacity(t *testing.T) {
 	factory := &mockObjectFactory{}
 	createPool(factory, 1, 10)
 	time.Sleep(time.Second)
-	assert.Equal(t, 0, factory.totalCreated)
-	assert.Equal(t, 1, factory.totalDisplosed)
+	assert.Equal(t, 0, factory.getTotalCreated())
+	assert.Equal(t, 1, factory.getTotalDisposed())
 }
 
 func TestAcquireIdle(t *testing.T) {
 	factory := &mockObjectFactory{}
 	pool := createPool(factory, 3, 0)
 	_, err := pool.Acquire(context.Background(), "")
 	assert.Nil(t, err)
-	assert.Equal(t, 0, factory.totalCreated)
+	assert.Equal(t, 0, factory.getTotalCreated())
 }
 func TestAcquireNonExists(t *testing.T) {
 	factory := &mockObjectFactory{}
 	pool := createPool(factory, 3, 0)
 	_, err := pool.Acquire(context.Background(), "1000")
 	assert.Nil(t, err)
-	assert.Equal(t, 0, factory.totalCreated)
+	assert.Equal(t, 0, factory.getTotalCreated())
 }
 
 func TestAcquireExists(t *testing.T) {
 	factory := &mockObjectFactory{}
 	pool := createPool(factory, 3, 0)
 	res, err := pool.Acquire(context.Background(), "2")
 	assert.Nil(t, err)
-	assert.Equal(t, 0, factory.totalCreated)
+	assert.Equal(t, 0, factory.getTotalCreated())
 	assert.Equal(t, "2", res.GetResourceID())
 }
 
@@ -176,7 +188,7 @@ func TestConcurrencyAcquireMoreThanCapacity(t *testing.T) {
 		}()
 	}
 	wg.Wait()
-	assert.Equal(t, 7, factory.totalCreated)
+	assert.Equal(t, 7, factory.getTotalCreated())
 }
 
 func TestRelease(t *testing.T) {
@@ -190,19 +202,19 @@ func TestRelease(t *testing.T) {
 	n4, _ := pool.Acquire(context.Background(), "")
 	n5, _ := pool.Acquire(context.Background(), "")
 	n6, _ := pool.Acquire(context.Background(), "")
-	assert.Equal(t, 3, factory.totalCreated)
+	assert.Equal(t, 3, factory.getTotalCreated())
 	pool.Release(n1.GetResourceID())
 	pool.Release(n2.GetResourceID())
 	pool.Release(n3.GetResourceID())
 	time.Sleep(1 * time.Second)
-	assert.Equal(t, 0, factory.totalDisplosed)
+	assert.Equal(t, 0, factory.getTotalDisposed())
 	pool.Release(n4.GetResourceID())
 	pool.Release(n5.GetResourceID())
 	time.Sleep(1 * time.Second)
-	assert.Equal(t, 0, factory.totalDisplosed)
+	assert.Equal(t, 0, factory.getTotalDisposed())
 	pool.Release(n6.GetResourceID())
 	time.Sleep(1 * time.Second)
-	assert.Equal(t, 1, factory.totalDisplosed)
+	assert.Equal(t, 1, factory.getTotalDisposed())
 }
 
 func TestReleaseInvalid(t *testing.T) {