Yiling-J / theine-go

high performance in-memory cache
MIT License
273 stars 17 forks source link

dead lock when setting a key that was deleted #37

Closed dctalbot closed 8 months ago

dctalbot commented 8 months ago

Hi, I'm seeing a dead lock when I try to use this package.

Code to reproduce:

go run main.go

package main

import (
    "fmt"
    "strings"
    "time"

    "github.com/Yiling-J/theine-go"
)

func isCollectionKey(key string) bool {
    return strings.Contains(key, ":")
}

func getCollectionName(key string) string {
    return strings.Split(key, ":")[0]
}

type Cache struct {
    tcache *theine.Cache[string, []byte]
}

func (c *Cache) evictCollection(name string) {
    c.tcache.Range(func(k string, v []byte) bool {
        if getCollectionName(k) == name {
            fmt.Println("evicting", k) // print eviction notice
            c.tcache.Delete(k)
        }
        return true
    })
}

func (c *Cache) Init() {
    cache, _ := theine.NewBuilder[string, []byte](2000).RemovalListener(func(k string, v []byte, r theine.RemoveReason) {

        // when a collection path expires
        if isCollectionKey(k) && r == theine.EXPIRED {

            // remove all entries for said collection
            c.evictCollection(getCollectionName(k))
        }

    }).Build()

    c.tcache = cache
}

func (c *Cache) Set(key string, value []byte) bool {
    return c.tcache.SetWithTTL(key, value, 0, 10*time.Second)
}

func main() {
    c := &Cache{}
    c.Init()
    c.Set("foo:bar", []byte("123"))
    time.Sleep(5 * time.Second)
    c.Set("foo:baz", []byte("123"))
    time.Sleep(10 * time.Second)    // expect to see foo:baz eviction notice
    c.Set("foo:baz", []byte("123")) // fatal error: all goroutines are asleep - deadlock!
}
Yiling-J commented 8 months ago

The Range method acquires a lock on each shard and releases it only after the callback function finishes execution. However, the Delete method attempts to acquire a lock on the same shard within the callback function, creating a deadlock because the Range method's lock is still held.

Try delaying the deletion operation until the Range method completes:

func (c *Cache) evictCollection(name string) {
    shouldDelete := []string{}
    c.tcache.Range(func(k string, v []byte) bool {
        if getCollectionName(k) == name {
            fmt.Println("evicting", k) // print eviction notice
            shouldDelete = append(shouldDelete, k)
            // c.tcache.Delete(k)
        }
        return true
    })
    for _, key := range shouldDelete {
        c.tcache.Delete(key)
    }
}
dctalbot commented 8 months ago

That seems to have been the issue. Thank you for the quick reply!