The OnEvicted function is not called if a value is re-set after expiration but before deletion

[vic4hub]

Hello, in my testing, my simple function which prints integer counts for a key to be evicted (set via .OnEvicted) is not happening some of the time, despite eviction happening as expected. I am using GO 1.7

[patrickmn]

Please provide an example of failing code.

[vic4hub]

package main

import (
	"time"
	"log"
	"github.com/patrickmn/go-cache"
)

func main() {

	gc := cache.New(5 * time.Second, 3 * time.Second)
	gc.OnEvicted(func(k string, v interface{}) {
		log.Println("evicting key: ", k, " value: ", v.(int))
	})
	Write(gc)
	time.Sleep(1 * time.Hour)
}

func Write(gc *cache.Cache) {
	for i := 1; i <= 40; i++ {
		cnt , itemErr := gc.IncrementInt("key1", 1);
		if  itemErr != nil{
			println(itemErr.Error())
			gc.Set("key1", 1, 5 * time.Second)
		}else{
			log.Println(cnt)
		}
		time.Sleep(300 * time.Millisecond)
	}
}

[patrickmn]

What exactly are you expecting to see, and what happens instead?

[vic4hub]

When provided, it is expected for eviction function to run every time key:val pair is evicted - however, if you run my code, you will see it does not happen.

[patrickmn]

This is because eviction happens during the cleanup, but the value is re-set after the item has expired and before the cleanup has run.

I agree this is a bug.

[vic4hub]

I see where it happens, got to change increment/decrement logic from "if !found || v.Expired() {" to just "if !found" throughout

[patrickmn]

You don't want to return values that have expired, though. Probably the fix is to check if a value exists but has expired in the Set functions, rather than just overwriting immediately (resulting in the evictor not seeing the old value and therefore not calling the OnExpired function on it.)

[jerrypeng]

@patrickmn any updates on getting this fixed?

[ayufan]

We made that fix #132 for this issue :)