From 8ab36d1d19fb41ad38f4181fa7d7b5bf60530505 Mon Sep 17 00:00:00 2001 From: Leonid Titov Date: Sun, 30 Oct 2022 19:54:55 +0300 Subject: [PATCH] fix concurrency --- cache.go | 39 ++++++++++++++++----------------------- options.go | 20 +++++--------------- 2 files changed, 21 insertions(+), 38 deletions(-) diff --git a/cache.go b/cache.go index 56c5e10..8f07b88 100644 --- a/cache.go +++ b/cache.go @@ -78,7 +78,7 @@ func New[K comparable, V any](opts ...Option[K, V]) *Cache[K, V] { // it to the cache and then returns it. If an item associated with the // provided key already exists, the new item overwrites the existing one. func (c *Cache[K, V]) Set(key K, value V) *Item[K, V] { - if !c.options.lockingFromOutside { + if !c.options.lockingDisabledForTransaction { c.CacheItems.Mu.Lock() defer c.CacheItems.Mu.Unlock() } @@ -90,7 +90,7 @@ func (c *Cache[K, V]) Set(key K, value V) *Item[K, V] { // it to the cache and then returns it. If an item associated with the // provided key already exists, the new item overwrites the existing one. func (c *Cache[K, V]) SetWithTTL(key K, value V, ttl time.Duration) *Item[K, V] { - if !c.options.lockingFromOutside { + if !c.options.lockingDisabledForTransaction { c.CacheItems.Mu.Lock() defer c.CacheItems.Mu.Unlock() } @@ -103,7 +103,7 @@ func (c *Cache[K, V]) SetWithTTL(key K, value V, ttl time.Duration) *Item[K, V] // provided key already exists, the new item overwrites the existing one. // DOES NOT UPDATE EXPIRATIONS func (c *Cache[K, V]) SetDontTouch(key K, value V) *Item[K, V] { - if !c.options.lockingFromOutside { + if !c.options.lockingDisabledForTransaction { c.CacheItems.Mu.Lock() defer c.CacheItems.Mu.Unlock() } @@ -116,7 +116,7 @@ func (c *Cache[K, V]) SetDontTouch(key K, value V) *Item[K, V] { // provided key already exists, the new item overwrites the existing one. // DOES NOT UPDATE EXPIRATIONS func (c *Cache[K, V]) SetWithTTLDontTouch(key K, value V, ttl time.Duration) *Item[K, V] { - if !c.options.lockingFromOutside { + if !c.options.lockingDisabledForTransaction { c.CacheItems.Mu.Lock() defer c.CacheItems.Mu.Unlock() } @@ -136,11 +136,11 @@ func (c *Cache[K, V]) Get(key K, opts ...Option[K, V]) *Item[K, V] { applyOptions(&getOpts, opts...) - if !c.options.lockingFromOutside { + if !c.options.lockingDisabledForTransaction { c.CacheItems.Mu.Lock() } elem := c.get(key, !getOpts.disableTouchOnHit) - if !c.options.lockingFromOutside { + if !c.options.lockingDisabledForTransaction { c.CacheItems.Mu.Unlock() } @@ -164,25 +164,18 @@ func (c *Cache[K, V]) Get(key K, opts ...Option[K, V]) *Item[K, V] { } func (c *Cache[K, V]) Transaction(f func(c *Cache[K, V])) { - lfoSaved := c.options.lockingFromOutside - - if !c.options.lockingFromOutside { - c.options.lockingFromOutside = true // prevent locks from inside the transaction - c.CacheItems.Mu.Lock() - defer c.CacheItems.Mu.Unlock() - } else { - // expecting already been locked from outside - } + c.CacheItems.Mu.Lock() + defer c.CacheItems.Mu.Unlock() + c.options.lockingDisabledForTransaction = true + defer func() { c.options.lockingDisabledForTransaction = false }() f(c) - - c.options.lockingFromOutside = lfoSaved } // Delete deletes an item from the cache. If the item associated with // the key is not found, the method is no-op. func (c *Cache[K, V]) Delete(key K) { - if !c.options.lockingFromOutside { + if !c.options.lockingDisabledForTransaction { c.CacheItems.Mu.Lock() defer c.CacheItems.Mu.Unlock() } @@ -197,18 +190,18 @@ func (c *Cache[K, V]) Delete(key K) { // DeleteAll deletes all items from the cache. func (c *Cache[K, V]) DeleteAll() { - if !c.options.lockingFromOutside { + if !c.options.lockingDisabledForTransaction { c.CacheItems.Mu.Lock() } c.evict(EvictionReasonDeleted) - if !c.options.lockingFromOutside { + if !c.options.lockingDisabledForTransaction { c.CacheItems.Mu.Unlock() } } // DeleteExpired deletes all expired items from the cache. func (c *Cache[K, V]) DeleteExpired() { - if !c.options.lockingFromOutside { + if !c.options.lockingDisabledForTransaction { c.CacheItems.Mu.Lock() defer c.CacheItems.Mu.Unlock() } @@ -234,11 +227,11 @@ func (c *Cache[K, V]) DeleteExpired() { // Its main purpose is to extend an item's expiration timestamp. // If the item is not found, the method is no-op. func (c *Cache[K, V]) Touch(key K) { - if !c.options.lockingFromOutside { + if !c.options.lockingDisabledForTransaction { c.CacheItems.Mu.Lock() } c.get(key, true) - if !c.options.lockingFromOutside { + if !c.options.lockingDisabledForTransaction { c.CacheItems.Mu.Unlock() } } diff --git a/options.go b/options.go index b01594e..f4fa8df 100644 --- a/options.go +++ b/options.go @@ -17,11 +17,11 @@ func (fn optionFunc[K, V]) apply(opts *options[K, V]) { // options holds all available cache configuration options. type options[K comparable, V any] struct { - capacity uint64 - ttl time.Duration - loader Loader[K, V] - disableTouchOnHit bool - lockingFromOutside bool + capacity uint64 + ttl time.Duration + loader Loader[K, V] + disableTouchOnHit bool + lockingDisabledForTransaction bool } // applyOptions applies the provided option values to the option struct. @@ -66,13 +66,3 @@ func WithDisableTouchOnHit[K comparable, V any]() Option[K, V] { opts.disableTouchOnHit = true }) } - -// With this set, the cache becomes thread-unsafe, and must be locked -// explicitly from outside. This is useful when external goroutine wrapper -// need to do atomic read-update transactions on values. -// NOTE: though you can use this, maybe you need to instead use the Transaction() method. -func WithLockingFromOutside[K comparable, V any]() Option[K, V] { - return optionFunc[K, V](func(opts *options[K, V]) { - opts.lockingFromOutside = true - }) -}