Lock slice before reading and modifying it

Question

My experience working with Go is recent and in reviewing some code, I have seen that while it is write-protected, there is a problem with reading the data. Not with the reading itself, but with possible modifications that can occur between the reading and the modification of the slice.

type ConcurrentSlice struct {
    sync.RWMutex
    items []Item
}

type Item struct {
    Index int
    Value Info
}

type Info struct {
    Name        string 
    Labels      map[string]string
    Failure     bool

}

As mentioned, the writing is protected in this way:

func (cs *ConcurrentSlice) UpdateOrAppend(item ScalingInfo) {
    found := false
    i := 0
    for inList := range cs.Iter() {
        if item.Name == inList.Value.Name{
            cs.items[i] = item
            found = true
        }
        i++
    }
    if !found {
        cs.Lock()
        defer cs.Unlock()

        cs.items = append(cs.items, item)
    }
}

func (cs *ConcurrentSlice) Iter() <-chan ConcurrentSliceItem {
    c := make(chan ConcurrentSliceItem)

    f := func() {
        cs.Lock()
        defer cs.Unlock()
        for index, value := range cs.items {
            c <- ConcurrentSliceItem{index, value}
        }
        close(c)
    }
    go f()

    return c
}

But between collecting the content of the slice and modifying it, modifications can occur.It may be that another routine modifies the same slice and when it is time to assign a value, it no longer exists: slice[i] = item

What would be the right way to deal with this?

I have implemented this method:

func GetList() *ConcurrentSlice {
    if list == nil {
        denylist = NewConcurrentSlice()
        return denylist
    }
    return denylist
}

And I use it like this:

concurrentSlice := GetList()
concurrentSlice.UpdateOrAppend(item)

But I understand that between the get and the modification, even if it is practically immediate, another routine may have modified the slice. What would be the correct way to perform the two operations atomically? That the slice I read is 100% the one I modify. Because if I try to assign an item to a index that no longer exists, it will break the execution.

Thank you in advance!

Answer 1

The way you are doing the blocking is incorrect, because it does not ensure that the items you return have not been removed. In case of an update, the array would still be at least the same length.

A simpler solution that works could be the following:

func (cs *ConcurrentSlice) UpdateOrAppend(item ScalingInfo) {
    found := false
    i := 0
    cs.Lock()
    defer cs.Unlock()

    for _, it := range cs.items {
        if item.Name == it.Name{
            cs.items[i] = it
            found = true
        }
        i++
    }
    if !found {
        cs.items = append(cs.items, item)
    }
}