Unexpected behavior in code using sync/atomic package for synchronization

Issue

Below is an example i was working on when learning about goroutines in Golang. In the code below we spawn 30 goroutines each of which accesses a shared variable called ordersProcessed. The example represents a cashier processing orders. Once ordersProcessed is more than 10 we print that the cashier is not able to take any more orders.

package main

import (
    "fmt"
    "sync"
    "sync/atomic"
)

func main() {
    var (
      wg sync.WaitGroup
      ordersProcessed int64
    )
    // This does not work as expected
    cashier := func(orderNum int) {
        value := atomic.LoadInt64(&ordersProcessed)
        fmt.Println("Value is ", value)
        if value < 10 {
            // Cashier is ready to serve!
            fmt.Println("Proessing order", orderNum)
            atomic.AddInt64(&ordersProcessed, 1)
        } else {
            // Cashier has reached the max capacity of processing orders.
            fmt.Println("I am tired! I want to take rest!", orderNum)
        }
        wg.Done()
    }

    for i := 0; i < 30; i++ {
        wg.Add(1)
        go func(orderNum int) {
            // Making an order
            cashier(orderNum)
        }(i)

    }
    wg.Wait()
}

Im expecting to see processed messages for 10 orders and unable to process henceforth. However, all the 30 orders get processed. I have used the sync/atomic package to synchronize the access to the ordersProcessed variable, however its value is always read as 0 by every goroutine. If however i change the code above to use a mutex as below, it works as expected:

package main

import (
    "fmt"
    "sync"
)

func main() {
    var (
       wg sync.WaitGroup
       ordersProcessed int64
       mutex sync.Mutex
    )
    // This works as expected
    cashier := func(orderNum int) {
        mutex.Lock()
        if ordersProcessed < 10 {
            // Cashier is ready to serve!
            fmt.Println("Processing order", orderNum)
            ordersProcessed++
        } else {
            // Cashier has reached the max capacity of processing orders.
            fmt.Println("I am tired! I want to take rest!", orderNum)
        }
        mutex.Unlock()
        wg.Done()
    }

    for i := 0; i < 30; i++ {
        wg.Add(1)
        go func(orderNum int) {
            // Making an order
            cashier(orderNum)
        }(i)

    }
    wg.Wait()
}

Can someone please tell me whats wrong with the way i used the sync/atomic package to synchronize access to the ordersProcessed variable ?

Solution

You used sync/atomic package, but you did not synchronize the goroutines.

When you start 30 goroutines, each goroutine starts by reading the shared variable, and incrementing it. If all goroutines read the variable, they will all read 0. The problem here is that you did not prevent other goroutines to modify the variable while one goroutine is working on it. After your program runs, the shared variable can be any value between 10 and 30, depending on how goroutines interleave.

Your second implementation is correct, that it prevents other goroutines from reading and modifying the shared variable while one of them is working on it.

Answered By – Burak Serdar

Answer Checked By – Timothy Miller (GoLangFix Admin)

Leave a Reply

Your email address will not be published.