Should we synchronize variable assignment in goroutine?

Question

Let's assume I declared two maps and want to assign it in two different goroutines in error group. I don't perform any reads/write. Should I protect assign operation with lock or I can omit it?

UPD3: In the Java Concurrency In Practice Brian Goetz's Part I Chapter 3 Shared Objects, mentioned:

Locking is not just about mutual exclusion; it is also memory visibility. To ensure that all threads see the most up-to-date values of shared mutable variables, the reading and writing threads must synchronize on a common lock.

var (
    mu     sync.Mutex
    one    map[string]struct{}
    two    map[string]struct{}
)
g, gctx := errgroup.WithContext(ctx)
g.Go(func() error {
    resp, err := invokeFirstService(gctx, request)
    if err != nil {
        return err
    }
    mu.Lock()
    one = resp.One
    mu.Unlock()
    return nil
})

g.Go(func() error {
    resp, err := invokeSecondService(gctx, request)
    if err != nil {
        return err
    }
    mu.Lock()
    two = resp.Two
    mu.Unlock()
    return nil
})
if err := g.Wait(); err != nil {
    return err
}
// UPD3: added lock and unlock section
m.Lock()
defer m.Unlock()
performAction(one, two)

UPD: added more context about variables

UPD2: what were my doubts: we have 3 goroutines - parent and two in the error group. there is no guarantee that our parent goroutine shared memory gets the last update after errgroup goroutines complete until we wrap access to shared memory with memory barriers

icza · Answer 1 · 2019-03-19T10:09:09.243

4

Group.Wait() blocks until all function calls from the Group.Go() method have returned, so this is a synchronization point. This ensures performAction(one, two) will not start before any writes to one and two are done, so in your example the mutex is unnecessary.

g, gctx := errgroup.WithContext(ctx)
g.Go(func() error {
    // ...
    one = resp.One
    return nil
})

g.Go(func() error {
    // ...
    two = resp.Two
    return nil
})

if err := g.Wait(); err != nil {
    return err
}
// Here you can access one and two safely:
performAction(one, two)

If you would access one and two from other goroutines while the goroutines that write them run concurrently, then yes, you would need to lock them, e.g.:

// This goroutine runs concurrently, so all concurrent access must be synchronized:
go func() {
    mu.Lock()
    fmt.Println(one, two)
    mu.Unlock()
}()

g, gctx := errgroup.WithContext(ctx)
g.Go(func() error {
    // ...
    mu.Lock()
    one = resp.One
    mu.Unlock()
    return nil
})

g.Go(func() error {
    // ...
    mu.Lock()
    two = resp.Two
    mu.Unlock()
    return nil
})

if err := g.Wait(); err != nil {
    return err
}
// Note that you don't need to lock here
// if the first concurrent goroutine only reads one and two.
performAction(one, two)

Also note that in the above example you could use sync.RWMutex, and in the goroutine that reads them, RWMutex.RLock() and RWMutex.RUnlock() would also be sufficient.

edited Mar 19 '19 at 10:09

answered Mar 19 '19 at 09:39

icza

389,944
63
907
827

what was my doubts: we have 3 goroutines: parent and two in the error group. there is no guarantee that our parent goroutine shared memory gets last update after errgroup goroutines complete until we wrap access to shared memory with memory barriers – Sergii Getman Mar 19 '19 at 12:54
1

@SergiiGetman If you use synchronization, you have your guarantee. `sync.Mutex` is one tool, so is `Group` or `WaitGroup`. Obviously 1 tool is enough, you don't have to use multiple of them. Read [The Go Memory Model](https://golang.org/ref/mem) for details. – icza Mar 19 '19 at 12:57
I've update question with adding lock/unlock on reading in code example and remark about: `reading and writing threads must synchronize on a common lock`. I understand that is java related remark, but I suggest it is applicable to golang as well. If it is doesn't related, correct me please – Sergii Getman Mar 24 '19 at 11:17
1

@SergiiGetman This is not applicable directly to Go. In Go, you need to estabilish a _happens-before relationship_. In the code of the question there is such a relationship: the `g.Wait()` call will happen before the `performAction()` call (`g.Wait()` will not return until its goroutines run), so the writes in the goroutines are guaranteed to happen and made visible before `performAction()` is called, or more precisely its arguments are evaluated. This happens-before relationship gives you the guarantee, as described in The Go Memory Model (which I suggested to read before). – icza Mar 24 '19 at 13:42
thank you very much for your explanation! unfortunately, it still looks complicated to me. In the links you have provided is the chapter Goroutine destruction: If the effects of a goroutine must be observed by another goroutine, use a synchronization mechanism such as a lock or channel communication to establish a relative ordering. That’s why I’m still confused. – Sergii Getman Mar 24 '19 at 14:00
@SergiiGetman Error group is another synchronization tool. So you're already using a synchronization mechanism. – icza Mar 24 '19 at 14:36
thanks! looks I got it. I also find a similar question on stack https://stackoverflow.com/questions/45894997/does-waitgroup-wait-imply-memory-barrier-in-this-case – Sergii Getman Mar 25 '19 at 12:37

score 0 · Answer 2 · answered Mar 19 '19 at 09:33

0

In this case, only one goroutine can access the map. I don't think you need a lock.

answered Mar 19 '19 at 09:33

Pan Ke

579
2
7

Should we synchronize variable assignment in goroutine?

2 Answers2

Linked