Go test hangs when using Mutex Lock and Unlock

Question

I am a newbie in GO and I had to use Mutex Lock/Unlock in my code to prevent concurrent access. But after I added the Lock/Unlock to my code, my tests started to run forever. I have simplified my use case and added the class and its test file. If I run the tests individually, all run fine. But if I run for the entire file, the first two are done, but the 3rd one runs forever. If I remove the Lock/Unlock, then the run are executed properly.

It would be great if someone could point out what I am doing wrong here.

Code under test:

package anything

import (
    "sync"
)

var maxLimit = 5
var Store = Cache{make([]string, 0)}
var mutex = new(sync.RWMutex)

type Cache struct {
    items []string
}

func (cache *Cache) Push(item string) {
    mutex.Lock()
    if len(cache.items) == maxLimit {
        cache.items = append(cache.items[:0], cache.items[1:]...)
    }
    cache.items = append(cache.items, item)
    mutex.Unlock()
}

func (cache *Cache) Get(content string) string {
    mutex.RLock()
    for _, item := range cache.items {
        if item == content {
            return content
        }
    }
    mutex.RUnlock()
    return ""
}

Test File:

package anything

import (
    "github.com/stretchr/testify/assert"
    "strconv"
    "testing"
)

func TestPush_MoreThanTheMaxLimit_RemoveFirstItem(t *testing.T) {
    for i := 0; i <= maxLimit; i++ {
        item := strconv.Itoa(i)
        Store.Push(item)
    }
    var actual = Store.items[0]
    assert.Equal(t, "1", actual)
}

func TestGet_PresentInCache_ReturnsItem(t *testing.T) {
    Store.Push(strconv.Itoa(1))
    Store.Push(strconv.Itoa(3))

    var actual = Store.Get("1")
    assert.Equal(t, "1", actual)
}

func TestGet_NotPresentInCache_ReturnsNil(t *testing.T) {
    Store.Push(strconv.Itoa(1))
    Store.Push(strconv.Itoa(3))

    var actual = Store.Get("7")
    assert.Empty(t, actual)
}

icza · Accepted Answer · 2021-03-24T15:49:36.887

If the Cache.Get() method finds the item, it returns without calling mutex.RUnlock(), so the mutex remains locked. Then calling Cache.Get() again will block because you can't lock an already locked mutex.

Instead unlock using defer, so the mutex will get unlocked no matter how the function ends:

func (cache *Cache) Get(content string) string {
    mutex.RLock()
    defer mutex.RUnlock()
    for _, item := range cache.items {
        if item == content {
            return content
        }
    }
    return ""
}

Also consider adding the mutex to the Cache struct itself, because that mutex is ought to protect concurrent access to a Cache value. It's fine in your example, but if you were to create multiple values of Cache, a single mutex would be insufficient (would unnecessarily block access to all Cache values while it would be enough to block access to a single Cache being accessed). It's also a good idea to embed the mutex, see When do you embed mutex in struct in Go?

Silly mistake :( Totally forgot about the return statement. Thanks for pointing out the usage of `defer`. — Henry, Mar 24 '21 at 15:56

Go test hangs when using Mutex Lock and Unlock

1 Answers1