Making a struct thread safe using go channels

Question

Suppose I have the following struct:

package manager

type Manager struct {
    strings []string
}

func (m *Manager) AddString(s string) {
    m.strings = append(m.strings, s)
}

func (m *Manager) RemoveString(s string) {
    for i, str := range m.strings {
        if str == s {
            m.strings = append(m.strings[:i], m.strings[i+1:]...)
        }
    }
}

This pattern is not thread safe, so the following test fails due to some race condition (array index out of bounds):

func TestManagerConcurrently(t *testing.T) {
    m := &manager.Manager{}
    wg := sync.WaitGroup{}
    for i:=0; i<100; i++ {
        wg.Add(1)
        go func () {
            m.AddString("a")
            m.AddString("b")
            m.AddString("c")
            m.RemoveString("b")
            wg.Done()
        } ()
    }
    wg.Wait()

    fmt.Println(m)
}

I'm new to Go, and from googling around I suppose I should use channels (?). So one way to make this concurrent would be like this:

type ManagerA struct {
    Manager
    addStringChan chan string
    removeStringChan chan string
}

func NewManagerA() *ManagerA {
    ma := &ManagerA{
        addStringChan: make(chan string),
        removeStringChan: make(chan string),
    }
    go func () {
        for {
            select {
            case msg := <-ma.addStringChan:
                ma.AddString(msg)
            case msg := <-ma.removeStringChan:
                ma.RemoveString(msg)
            }
        }
    }()
    return ma
}

func (m* ManagerA) AddStringA(s string) {
    m.addStringChan <- s
}
func (m* ManagerA) RemoveStringA(s string) {
    m.removeStringChan <- s
}

I would like to expose an API similar to the non-concurrent example, hence AddStringA, RemoveStringA.

This seems to work as expected concurrently (although I guess the inner goroutine should also exit at some point). My problem with this is that there is a lot of extra boilerplate:

• need to define & initialize channels
• define inner goroutine loop with select
• map functions to channel calls

It seems a bit much to me. Is there a way to simplify this (refactor / syntax / library)?

I think the best way to implement this would be to use a Mutex instead? But is it still possible to simplify this sort of boilerplate?

Answer 1

Using a mutex would be perfectly idiomatic like this:

type Manager struct {
    mu      sync.Mutex
    strings []string
}

func (m *Manager) AddString(s string) {
    m.mu.Lock()
    m.strings = append(m.strings, s)
    m.mu.Unlock()
}

func (m *Manager) RemoveString(s string) {
    m.mu.Lock()
    for i, str := range m.strings {
        if str == s {
            m.strings = append(m.strings[:i], m.strings[i+1:]...)
        }
    }
    m.mu.Unlock()
}

You could do this with channels, but as you noted it is a lot of extra work for not much gain. Just use a mutex is my advice!

Answer 2

If you simply need to make the access to the struct thread-safe, use mutex:

type Manager struct {
   sync.Mutex
   data []string
}

func (m *Manager) AddString(s string) {
    m.Lock()
    m.strings = append(m.strings, s)
    m.Unlock()
}