Skip to content

Commit 0c0a353

Browse files
committed
Refactor DeleteExpired for safer behavior
- make sure that expManager queue is not empty before popping, which can happen if an explicit delete happens while DeleteExpired releases the lock. - evict() now returns shouldContinue instead of a shouldBreak to simplify the loop.
1 parent 274db6d commit 0c0a353

File tree

1 file changed

+10
-13
lines changed

1 file changed

+10
-13
lines changed

cache.go

+10-13
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ func (item *Item[K, V]) hasExpiration() bool {
5050
return !item.Expiration.IsZero()
5151
}
5252

53+
//Print tries to make a readable log of the item
54+
5355
// Expired returns true if the item has expired.
5456
func (item *Item[K, V]) Expired() bool {
5557
if !item.hasExpiration() {
@@ -241,31 +243,26 @@ func (c *Cache[K, V]) GetOrSet(key K, val V, opts ...ItemOption) (actual V, load
241243

242244
// DeleteExpired all expired items from the cache.
243245
func (c *Cache[K, V]) DeleteExpired() {
244-
c.mu.Lock()
245-
l := c.expManager.len()
246-
c.mu.Unlock()
247-
248246
evict := func() bool {
247+
c.mu.Lock()
248+
defer c.mu.Unlock()
249+
if c.expManager.len() == 0 {
250+
return false
251+
}
249252
key := c.expManager.pop()
250253
// if is expired, delete it and return nil instead
251254
item, ok := c.cache.Get(key)
252255
if ok {
253256
if item.Expired() {
254257
c.cache.Delete(key)
255-
return false
258+
return true
256259
}
257260
c.expManager.update(key, item.Expiration)
258261
}
259-
return true
262+
return false
260263
}
261264

262-
for i := 0; i < l; i++ {
263-
c.mu.Lock()
264-
shouldBreak := evict()
265-
c.mu.Unlock()
266-
if shouldBreak {
267-
break
268-
}
265+
for evict() {
269266
}
270267
}
271268

0 commit comments

Comments
 (0)