go-cache icon indicating copy to clipboard operation
go-cache copied to clipboard

Using testing/synctest on cache with janitor goroutine will hang forever

Open cyrilc-pro opened this issue 10 months ago • 1 comments

When using the new experimental synctest feature of Go 1.24, I observe the following.

This test runs just fine:

func TestCacheWithoutJanitor(t *testing.T) {
	synctest.Run(func() {
		c := cache.New(time.Second, cache.NoExpiration)
		c.Set("foo", "bar", cache.DefaultExpiration)
	})
}

This test hangs forever:

func TestCacheWithJanitor(t *testing.T) {
	synctest.Run(func() {
		c := cache.New(time.Second, time.Minute)
		c.Set("foo", "bar", cache.DefaultExpiration)
	})
}

The incompatibility comes from:

  • synctest.Run will wait for all goroutines to exit (including the janitor goroutine)
  • cache uses runtime.SetFinalizer to stop the janitor goroutine but is never called

Forcing the GC to run produces a panic send on synctest channel from outside bubble:

func TestCacheWithJanitor(t *testing.T) {
	synctest.Run(func() {
		c := cache.New(time.Second, time.Minute)
		c.Set("foo", "bar", cache.DefaultExpiration)
		c = nil
		runtime.GC()
	})
}

My suggestion would be to provide a Close() function on the cache object to stop the janitor goroutine without the need for the GC to run.

cyrilc-pro avatar Feb 27 '25 15:02 cyrilc-pro

One of the difficulties with adding a Close() method is that the cache remains "valid" after calling it, except it's not really valid because the janitor is no longer working.

This is not so easy to fix well: Set() doesn't return an error, Get() returns (key, isset) and doesn't have a brilliant way to signal it, etc.

Perhaps Close() can just set cache.items to nil?

func (c *cache) Close() error {
    if c.janitor != nil {
        c.janitor.stop <- true
    }
    c.mu.Lock()
    defer c.mu.Unlock()
    c.items = nil
    return nil
}

That will at least panic. But meh...

arp242 avatar Mar 18 '25 06:03 arp242