On delete, always set promotions == -2 and node == nil
Also, item.promotions doesn't need to be loaded/stored using atomic. Once upon a time it did. Cache was updated long ago to not use atomic operations on it, but LayeredCache wasn't. They are both consistent now (they don't use atomic operations). Fixes: https://github.com/karlseguin/ccache/issues/76
This commit is contained in:
2
cache.go
2
cache.go
@@ -394,6 +394,8 @@ func (c *Cache[T]) doDelete(item *Item[T]) {
|
||||
c.onDelete(item)
|
||||
}
|
||||
c.list.Remove(item.node)
|
||||
item.node = nil
|
||||
item.promotions = -2
|
||||
}
|
||||
}
|
||||
|
||||
|
@@ -332,7 +332,7 @@ func Test_CachePrune(t *testing.T) {
|
||||
cache.Set(key, key, 5*time.Minute)
|
||||
}
|
||||
if epoch%500 == 0 {
|
||||
assert.True(t, cache.GetSize() < 500)
|
||||
assert.True(t, cache.GetSize() <= 500)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@@ -266,13 +266,15 @@ func (c *LayeredCache[T]) worker() {
|
||||
}
|
||||
deleteItem := func(item *Item[T]) {
|
||||
if item.node == nil {
|
||||
atomic.StoreInt32(&item.promotions, -2)
|
||||
item.promotions = -2
|
||||
} else {
|
||||
c.size -= item.size
|
||||
if c.onDelete != nil {
|
||||
c.onDelete(item)
|
||||
}
|
||||
c.list.Remove(item.node)
|
||||
item.node = nil
|
||||
item.promotions = -2
|
||||
}
|
||||
}
|
||||
for {
|
||||
@@ -318,13 +320,13 @@ func (c *LayeredCache[T]) worker() {
|
||||
|
||||
func (c *LayeredCache[T]) doPromote(item *Item[T]) bool {
|
||||
// deleted before it ever got promoted
|
||||
if atomic.LoadInt32(&item.promotions) == -2 {
|
||||
if item.promotions == -2 {
|
||||
return false
|
||||
}
|
||||
if item.node != nil { //not a new item
|
||||
if item.shouldPromote(c.getsPerPromote) {
|
||||
c.list.MoveToFront(item.node)
|
||||
atomic.StoreInt32(&item.promotions, 0)
|
||||
item.promotions = 0
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
@@ -391,7 +391,7 @@ func Test_LayeredCachePrune(t *testing.T) {
|
||||
cache.Set(key, key, key, 5*time.Minute)
|
||||
}
|
||||
if epoch%500 == 0 {
|
||||
assert.True(t, cache.GetSize() < 500)
|
||||
assert.True(t, cache.GetSize() <= 500)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
Reference in New Issue
Block a user