From 05f49114fcf4702463c7549e689ac4191b633f32 Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Sat, 2 Apr 2022 13:18:01 +0800 Subject: [PATCH 1/2] *: fix data race in the cache Signed-off-by: Weizhen Wang --- cache.go | 17 +++++++++-------- go.mod | 2 +- go.sum | 3 +++ 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/cache.go b/cache.go index 1b3b1a8d..cd239e5b 100644 --- a/cache.go +++ b/cache.go @@ -26,6 +26,7 @@ import ( "unsafe" "github.com/dgraph-io/ristretto/z" + "go.uber.org/atomic" ) var ( @@ -64,7 +65,7 @@ type Cache struct { // stop is used to stop the processItems goroutine. stop chan struct{} // indicates whether cache is closed. - isClosed bool + isClosed atomic.Bool // cost calculates cost from a value. cost func(value interface{}) int64 // ignoreInternalCost dictates whether to ignore the cost of internally storing @@ -214,7 +215,7 @@ func NewCache(config *Config) (*Cache, error) { } func (c *Cache) Wait() { - if c == nil || c.isClosed { + if c == nil || atomic.c.isClosed { return } wg := &sync.WaitGroup{} @@ -227,7 +228,7 @@ func (c *Cache) Wait() { // value was found or not. The value can be nil and the boolean can be true at // the same time. func (c *Cache) Get(key interface{}) (interface{}, bool) { - if c == nil || c.isClosed || key == nil { + if c == nil || c.isClosed.Load() || key == nil { return nil, false } keyHash, conflictHash := c.keyToHash(key) @@ -270,7 +271,7 @@ func (c *Cache) SetIfPresent(key, value interface{}, cost int64) bool { func (c *Cache) setInternal(key, value interface{}, cost int64, ttl time.Duration, onlyUpdate bool) bool { - if c == nil || c.isClosed || key == nil { + if c == nil || c.isClosed.Load() || key == nil { return false } @@ -326,7 +327,7 @@ func (c *Cache) setInternal(key, value interface{}, // Del deletes the key-value item from the cache if it exists. func (c *Cache) Del(key interface{}) { - if c == nil || c.isClosed || key == nil { + if c == nil || c.isClosed.load || key == nil { return } keyHash, conflictHash := c.keyToHash(key) @@ -373,7 +374,7 @@ func (c *Cache) GetTTL(key interface{}) (time.Duration, bool) { // Close stops all goroutines and closes all channels. func (c *Cache) Close() { - if c == nil || c.isClosed { + if c == nil || c.isClosed.Load() { return } c.Clear() @@ -383,14 +384,14 @@ func (c *Cache) Close() { close(c.stop) close(c.setBuf) c.policy.Close() - c.isClosed = true + c.isClosed.Store(true) } // Clear empties the hashmap and zeroes all policy counters. Note that this is // not an atomic operation (but that shouldn't be a problem as it's assumed that // Set/Get calls won't be occurring until after this). func (c *Cache) Clear() { - if c == nil || c.isClosed { + if c == nil || c.isClosed.Load() { return } // Block until processItems goroutine is returned. diff --git a/go.mod b/go.mod index f7a620c5..1f7e944b 100644 --- a/go.mod +++ b/go.mod @@ -4,11 +4,11 @@ go 1.12 require ( github.com/cespare/xxhash/v2 v2.1.1 - github.com/davecgh/go-spew v1.1.1 // indirect github.com/dgryski/go-farm v0.0.0-20190423205320-6a90982ecee2 github.com/dustin/go-humanize v1.0.0 github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b github.com/pkg/errors v0.9.1 github.com/stretchr/testify v1.4.0 + go.uber.org/atomic v1.9.0 golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f ) diff --git a/go.sum b/go.sum index 6e3d7903..52fa3454 100644 --- a/go.sum +++ b/go.sum @@ -14,8 +14,11 @@ github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= +go.uber.org/atomic v1.9.0 h1:ECmE8Bn/WFTYwEW/bpKD3M8VtR/zQVbavAoalC1PYyE= +go.uber.org/atomic v1.9.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f h1:+Nyd8tzPX9R7BWHguqsrbFdRx3WQ/1ib8I44HXV5yTA= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= From 7556ec01f9dbce2034a13ecc98b10ba709572dc3 Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Sat, 2 Apr 2022 13:29:34 +0800 Subject: [PATCH 2/2] fix --- cache.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cache.go b/cache.go index cd239e5b..7b1b0328 100644 --- a/cache.go +++ b/cache.go @@ -215,7 +215,7 @@ func NewCache(config *Config) (*Cache, error) { } func (c *Cache) Wait() { - if c == nil || atomic.c.isClosed { + if c == nil || c.isClosed.Load() { return } wg := &sync.WaitGroup{} @@ -327,7 +327,7 @@ func (c *Cache) setInternal(key, value interface{}, // Del deletes the key-value item from the cache if it exists. func (c *Cache) Del(key interface{}) { - if c == nil || c.isClosed.load || key == nil { + if c == nil || c.isClosed.Load() || key == nil { return } keyHash, conflictHash := c.keyToHash(key)