How to range over a slice of structs that contain Mutexes in Go

Question

I am experimenting with Go and trying a variety of things for concurrent state management in a server. Say we have the following:

type Resource struct {
    data int
}

func (r *Resource) increment () {
    r.data++
}

type Client struct {
    id       int
    resource Resource
    mu       sync.RWMutex
}

type ActiveClients struct {
    clients []Client
    mu      sync.RWMutex
}

func (ac *ActiveClients) add(client Client) {
    ac.mu.Lock()
    defer ac.mu.Unlock()
    if ac.clients == nil {
        ac.clients = make([]Client, 0)
    }
    ac.clients = append(ac.clients, client)
}

The ActiveClients.mu would be used for reading and writing to the ActiveClients.clients slice, and the Client.mu would be used for reading from and writing to the Client.resource. Now let's say we want to iterate over ActiveClients.clients to update one of the resources. The following creates an error:

func (ac *ActiveClients) addToResource(clientId int) {
    for _, existingClient := range ac.clients {
        if existingClient.id == clientId {
            existingClient.Lock()
            defer existingClient.Unlock()
            existingClient.resource.increment()
        }
    }
}

This produces "range var existingClient copies lock: {modulename}.Client contains sync.RWMutex".

How do I range over the slice without copying the lock?

Answer 1

The for _, v := range s statement assigns the elements of s to local variable v. The value is copied. There's no reference semantics

The go vet command warns you that the mutex field is copied. A mutex should not be copied once used.

That's not the only problem in incrementResource. The function modifies the copy of client in the local variable, not the client in the slice. Because the local variable is discarded on return from the function, the function incrementResource has no effect. Run the program https://go.dev/play/p/kL9GZSL6d2j to see a demonstration of the problem.

Fix the bugs in incrementResource by accessing the slice element through a pointer.

func (ac *ActiveClients) addToResource(clientId int) {
    for i := range ac.clients {
        existingClient := &ac.clients[i] // existingClient is ptr to slice element
        if existingClient.id == clientId {
            existingClient.Lock()
            defer existingClient.Unlock()
            existingClient.resource.increment()
            fmt.Println("data in addToResource: ", existingClient.resource.data)
        }
    }
}

Here's the program with the fix: https://go.dev/play/p/wMSUOjoTauB

The above change fixes the issue in the question, but that's not the only problem with the application. The call to append in method ActiveClients.add copies the Client values when growing the slice. This copy creates a data race on the slice elements and violates the rule that a mutex should not be copied once used.

To fix everything, use a slice of *Client instead of Client. While we are at it, take advantage of append's handling of nil slices.

type ActiveClients struct {
    clients []*Client
    mu      sync.RWMutex
}

func (ac *ActiveClients) add(client *Client) {
    ac.mu.Lock()
    defer ac.mu.Unlock()
    ac.clients = append(ac.clients, client)
}

Here's the final program: https://go.dev/play/p/miNK90ZDNCu