goconcurrencyconditional-variable

sync.Cond with Wait method in Go


I read unusual case in documentation sync.Cond:

Because c.L is not locked while Wait is waiting, the caller typically cannot assume that the condition is true when Wait returns. Instead, the caller should Wait in a loop:

c.L.Lock()
for !condition() {
    c.Wait()
}

... make use of condition ...
c.L.Unlock()

I don't understand the idea of it... For example in my code, do I need to have loop or not?

func subscribe(name string, data map[string]string, c *sync.Cond) {
    c.L.Lock()

    for len(data) != 0 {
        c.Wait()
    }

    fmt.Printf("[%s] %s\n", name, data["key"])

    c.L.Unlock()
}

func publish(name string, data map[string]string, c *sync.Cond) {
    time.Sleep(time.Second)

    c.L.Lock()
    data["key"] = "value"
    c.L.Unlock()

    fmt.Printf("[%s] data publisher\n", name)
    c.Broadcast()
}

func main() {
    data := map[string]string{}
    cond := sync.NewCond(&sync.Mutex{})

    wg := sync.WaitGroup{}
    wg.Add(3)

    go func() {
        defer wg.Done()
        subscribe("subscriber_1", data, cond)
    }()

    go func() {
        defer wg.Done()
        subscribe("subscriber_2", data, cond)
    }()

    go func() {
        defer wg.Done()
        publish("publisher", data, cond)
    }()

    wg.Wait()
}

Solution

  • Because c.L is not locked while Wait is waiting, the caller typically cannot assume that the condition is true when Wait returns. Instead, the caller should Wait in a loop:

    When you call Wait() on a sync.Cond, it releases the associated lock and suspends execution of the calling goroutine until another goroutine calls Signal or Broadcast on the same sync.Cond.

    When Wait returns (meaning it has been signalled), it re-acquires the lock before returning to the caller. However, since the lock was released while Wait was waiting, it's possible that the condition the caller was waiting for has changed between when Wait was called and when it returned.

    For example, let's say you have a shared buffer and multiple goroutines are reading from and writing to it. You might use a sync.Cond to wait until the buffer is not empty:

    c.L.Lock()
    for len(buffer) == 0 {
        c.Wait()
    }
    // ... read from buffer ...
    c.L.Unlock()
    

    You call Wait when the buffer is empty, expecting that it will return once another goroutine has added an item to the buffer. However, it's possible that after Wait returns and before your goroutine starts executing again, another goroutine could have already consumed the item from the buffer, leaving it empty again.

    Because of this, when Wait returns, you can't assume that len(buffer) != 0, even though that's the condition you were waiting for. Instead, you should check the condition again in a loop, as shown in the example code, to make sure it's still true before you proceed.

    The loop (for len(buffer) == 0) ensures that if the condition is not met when Wait returns, it will simply wait again until the condition is met.

    For example in my code, do I need to have loop or not?

    Yes, your subscribe function would need to have a loop.

    In your case, you are waiting until the length of data map becomes zero. However, your publisher function is adding an element to the map, so the condition len(data) != 0 will always be true. This would make the Wait function to be never triggered.

    However, if you were checking for a condition that might have been updated multiple times, a loop would be necessary. When Wait is called, it releases the lock and suspends the execution of the calling goroutine. Later, when another goroutine calls Broadcast or Signal, the Wait call returns and re-acquires the lock. At this point, the condition that the goroutine was waiting for might no longer be true, which is why you should typically call Wait in a loop.

    In a nutshell, in the current state of your code, loop is not needed because your condition (length of data map becomes zero) will never happen based on the code. But if you change the condition or add more logic, the usage of a loop can be required.

    If you change your publisher function to empty the map and you want to ensure that subscriber processes the data only when the map is empty, you would use a loop in this way:

    func subscribe(name string, data map[string]string, c *sync.Cond) {
        c.L.Lock()
        for len(data) != 0 {
            c.Wait()
        }
        // ... process data ...
        c.L.Unlock()
    }
    

    This loop will ensure that the data processing doesn't start until the map is empty. It will keep waiting if the map is not empty whenever Wait returns.