rework lru's external registered callback invocation to avoid concurr… (

#80)

* rework lru's external registered callback invocation to avoid concurrency issue such as possible deadlock

* fix lint issues

lru.go

@@ -6,10 +6,17 @@ import (
 	"github.com/hashicorp/golang-lru/simplelru"
 )
 
+const (
+	// DefaultEvictedBufferSize defines the default buffer size to store evicted key/val
+	DefaultEvictedBufferSize = 16
+)
+
 // Cache is a thread-safe fixed size LRU cache.
 type Cache struct {
-	lru  simplelru.LRUCache
-	lock sync.RWMutex
+	lru                      *simplelru.LRU
+	evictedKeys, evictedVals []interface{}
+	onEvictedCB              func(k, v interface{})
+	lock                     sync.RWMutex
 }
 
 // New creates an LRU of the given size.
@@ -19,30 +26,63 @@ func New(size int) (*Cache, error) {
 
 // NewWithEvict constructs a fixed size cache with the given eviction
 // callback.
-func NewWithEvict(size int, onEvicted func(key interface{}, value interface{})) (*Cache, error) {
-	lru, err := simplelru.NewLRU(size, simplelru.EvictCallback(onEvicted))
-	if err != nil {
-		return nil, err
+func NewWithEvict(size int, onEvicted func(key, value interface{})) (c *Cache, err error) {
+	// create a cache with default settings
+	c = &Cache{
+		onEvictedCB: onEvicted,
 	}
-	c := &Cache{
-		lru: lru,
+	if onEvicted != nil {
+		c.initEvictBuffers()
+		onEvicted = c.onEvicted
 	}
-	return c, nil
+	c.lru, err = simplelru.NewLRU(size, onEvicted)
+	return
+}
+
+func (c *Cache) initEvictBuffers() {
+	c.evictedKeys = make([]interface{}, 0, DefaultEvictedBufferSize)
+	c.evictedVals = make([]interface{}, 0, DefaultEvictedBufferSize)
+}
+
+// onEvicted save evicted key/val and sent in externally registered callback
+// outside of critical section
+func (c *Cache) onEvicted(k, v interface{}) {
+	c.evictedKeys = append(c.evictedKeys, k)
+	c.evictedVals = append(c.evictedVals, v)
 }
 
 // Purge is used to completely clear the cache.
 func (c *Cache) Purge() {
+	var ks, vs []interface{}
 	c.lock.Lock()
 	c.lru.Purge()
+	if c.onEvictedCB != nil && len(c.evictedKeys) > 0 {
+		ks, vs = c.evictedKeys, c.evictedVals
+		c.initEvictBuffers()
+	}
 	c.lock.Unlock()
+	// invoke callback outside of critical section
+	if c.onEvictedCB != nil {
+		for i := 0; i < len(ks); i++ {
+			c.onEvictedCB(ks[i], vs[i])
+		}
+	}
 }
 
 // Add adds a value to the cache. Returns true if an eviction occurred.
 func (c *Cache) Add(key, value interface{}) (evicted bool) {
+	var k, v interface{}
 	c.lock.Lock()
 	evicted = c.lru.Add(key, value)
+	if c.onEvictedCB != nil && evicted {
+		k, v = c.evictedKeys[0], c.evictedVals[0]
+		c.evictedKeys, c.evictedVals = c.evictedKeys[:0], c.evictedVals[:0]
+	}
 	c.lock.Unlock()
-	return evicted
+	if c.onEvictedCB != nil && evicted {
+		c.onEvictedCB(k, v)
+	}
+	return
 }
 
 // Get looks up a key's value from the cache.
@@ -75,61 +115,102 @@ func (c *Cache) Peek(key interface{}) (value interface{}, ok bool) {
 // recent-ness or deleting it for being stale, and if not, adds the value.
 // Returns whether found and whether an eviction occurred.
 func (c *Cache) ContainsOrAdd(key, value interface{}) (ok, evicted bool) {
+	var k, v interface{}
 	c.lock.Lock()
-	defer c.lock.Unlock()
-
 	if c.lru.Contains(key) {
+		c.lock.Unlock()
 		return true, false
 	}
 	evicted = c.lru.Add(key, value)
+	if c.onEvictedCB != nil && evicted {
+		k, v = c.evictedKeys[0], c.evictedVals[0]
+		c.evictedKeys, c.evictedVals = c.evictedKeys[:0], c.evictedVals[:0]
+	}
+	c.lock.Unlock()
+	if c.onEvictedCB != nil && evicted {
+		c.onEvictedCB(k, v)
+	}
 	return false, evicted
 }
 
 // PeekOrAdd checks if a key is in the cache without updating the
 // recent-ness or deleting it for being stale, and if not, adds the value.
 // Returns whether found and whether an eviction occurred.
 func (c *Cache) PeekOrAdd(key, value interface{}) (previous interface{}, ok, evicted bool) {
+	var k, v interface{}
 	c.lock.Lock()
-	defer c.lock.Unlock()
-
 	previous, ok = c.lru.Peek(key)
 	if ok {
+		c.lock.Unlock()
 		return previous, true, false
 	}
-
 	evicted = c.lru.Add(key, value)
+	if c.onEvictedCB != nil && evicted {
+		k, v = c.evictedKeys[0], c.evictedVals[0]
+		c.evictedKeys, c.evictedVals = c.evictedKeys[:0], c.evictedVals[:0]
+	}
+	c.lock.Unlock()
+	if c.onEvictedCB != nil && evicted {
+		c.onEvictedCB(k, v)
+	}
 	return nil, false, evicted
 }
 
 // Remove removes the provided key from the cache.
 func (c *Cache) Remove(key interface{}) (present bool) {
+	var k, v interface{}
 	c.lock.Lock()
 	present = c.lru.Remove(key)
+	if c.onEvictedCB != nil && present {
+		k, v = c.evictedKeys[0], c.evictedVals[0]
+		c.evictedKeys, c.evictedVals = c.evictedKeys[:0], c.evictedVals[:0]
+	}
 	c.lock.Unlock()
+	if c.onEvictedCB != nil && present {
+		c.onEvicted(k, v)
+	}
 	return
 }
 
 // Resize changes the cache size.
 func (c *Cache) Resize(size int) (evicted int) {
+	var ks, vs []interface{}
 	c.lock.Lock()
 	evicted = c.lru.Resize(size)
+	if c.onEvictedCB != nil && evicted > 0 {
+		ks, vs = c.evictedKeys, c.evictedVals
+		c.initEvictBuffers()
+	}
 	c.lock.Unlock()
+	if c.onEvictedCB != nil && evicted > 0 {
+		for i := 0; i < len(ks); i++ {
+			c.onEvictedCB(ks[i], vs[i])
+		}
+	}
 	return evicted
 }
 
 // RemoveOldest removes the oldest item from the cache.
 func (c *Cache) RemoveOldest() (key, value interface{}, ok bool) {
+	var k, v interface{}
 	c.lock.Lock()
 	key, value, ok = c.lru.RemoveOldest()
+	if c.onEvictedCB != nil && ok {
+		k, v = c.evictedKeys[0], c.evictedVals[0]
+		c.evictedKeys, c.evictedVals = c.evictedKeys[:0], c.evictedVals[:0]
+	}
 	c.lock.Unlock()
+	if c.onEvictedCB != nil && ok {
+		c.onEvictedCB(k, v)
+	}
 	return
 }
 
 // GetOldest returns the oldest entry
 func (c *Cache) GetOldest() (key, value interface{}, ok bool) {
-	c.lock.Lock()
+	c.lock.RLock()
 	key, value, ok = c.lru.GetOldest()
-	c.lock.Unlock()
+	c.lock.RUnlock()
 	return
 }