store_memory.go cache.Items()[key] causes panic in high concurrency situations

[dougnukem]

While testing #17 I ran into an occasional panic in the store_memory.go accessing the `cache.Items()[key]

=== RUN   TestConcurrency
panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xb code=0x1 addr=0x0 pc=0x5e304]

goroutine 555 [running]:
github.com/ulule/limiter.(*MemoryStore).Get(0xc82000bba0, 0xc820866920, 0x18, 0x0, 0x0, 0xa, 0x186a0, 0x0, 0x0, 0x0, ...)
    /Users/ddaniels/dev/src/github.com/ulule/limiter/store_memory.go:35 +0x24c
github.com/ulule/limiter.(*Limiter).Get(0xc820017b60, 0x4742b8, 0x4, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
    /Users/ddaniels/dev/src/github.com/ulule/limiter/limiter.go:35 +0x9f
github.com/ulule/limiter.TestConcurrency.func1(0xc820017b60, 0xc820020750, 0xc8200be4f0, 0x210)
    /Users/ddaniels/dev/src/github.com/ulule/limiter/limiter_test.go:96 +0x50
created by github.com/ulule/limiter.TestConcurrency
    /Users/ddaniels/dev/src/github.com/ulule/limiter/limiter_test.go:100 +0x200

The go-cache documentation says you shouldn't access Items() without synchronizing access (which I don't think is possible because the RWLock is private to the cache)

https://github.com/patrickmn/go-cache/blob/master/cache.go#L1001

// Returns the items in the cache. This may include items that have expired,
// but have not yet been cleaned up. If this is significant, the Expiration
// fields of the items should be checked. Note that explicit synchronization
// is needed to use a cache and its corresponding Items() return value at
// the same time, as the map is shared.
func (c *cache) Items() map[string]Item

I put up a PR to them to ensure that cache.Items() returns a copy of the map using the synchronized lock while doing so.

Also I added a method GetItem to retrieve a cache Item:

https://github.com/dougnukem/go-cache/blob/53c1a5bdb67be8eaf0beee21efdbf7db3bab734a/cache.go#L146

// Get a cache Item from the cache (includes Item.Expiration, Item.Object).
// Returns the item or nil, and a bool indicating
// whether the key was found.
func (c *cache) GetItem(k string) (Item, bool) {

[thoas]

thank you for your investigation @dougnukem, we will review it as soon as possible on our side :)

[dcelasun]

Any news on this?

[thoas]

@dcelasun no news, we are not available at that moment and we are not using the in-memory storage so we are accepting PRs :)