CODEWITHSUNDEEP
multithreadinggosynchronizationrace-conditiongoroutine
shengjiang
shengjiang

Reputation: 43

Why go func in Go function needs waitgroup to exit correctly?

Sry this title might be misleading. Actually the full code is here below:

package main

import (
    "fmt"
    "sync"
)

type Button struct {
    Clicked *sync.Cond
}

func main() {
    button := Button{
        Clicked: sync.NewCond(&sync.Mutex{}),
    }
    subscribe := func(c *sync.Cond, fn func()) {
        var wg sync.WaitGroup
        wg.Add(1)
        go func() {
            wg.Done()
            c.L.Lock()
            defer c.L.Unlock()
            c.Wait()
            fn()
        }()
        wg.Wait()
    }

    var clickRegistered sync.WaitGroup
    clickRegistered.Add(2)

    subscribe(button.Clicked, func() {
        fmt.Println("maximizing window")
        clickRegistered.Done()
    })
    subscribe(button.Clicked, func() {
        fmt.Println("displaying dialog")
        clickRegistered.Done()
    })

    button.Clicked.Broadcast()
    clickRegistered.Wait()
}

When I comment some lines and run it again, it throws a fatal error "all goroutines are asleep - deadlock!"
The subscribe function altered looks like as below:

subscribe := func(c *sync.Cond, fn func()) {
        //var wg sync.WaitGroup
        //wg.Add(1)
        go func() {
            //wg.Done()
            c.L.Lock()
            defer c.L.Unlock()
            c.Wait()
            fn()
        }()
        //wg.Wait()
    }

What makes me confused is that whether go func is executed before the outer subscribe function returns. In my thought, the go func will run as a daemon though the outer function has returned, so the wg variable is unnecessary. But it shows I'm totally wrong. So if the go func has the possibility of not being scheduled, does it mean that we must use the sync.WaitGroup in every function or code block to make sure the goroutine is scheduled to be executed before the function or code block returns?
Thank you all.

Upvotes: 0

Views: 351

Answers (2)

leaf bebop
leaf bebop

Reputation: 8212

The problem is that c.Wait() in either call is not guaranteed to run before button.Clicked.Broadcast(); and even your original code's use of WaitGroup does not guarantees it either (since it is the c.Wait() part, not the spawn of the goroutine that is important)

modified subscribe:

subscribe := func(c *sync.Cond, subWG *sync.WaitGroup, fn func()) {
        go func() {
            c.L.Lock()
            defer c.L.Unlock()
            subWG.Done() // [2]
            c.Wait()
            fn()
        }()
    }

code of waiting:

subWG.Done()
button.Clicked.L.Lock()
button.Clicked.L.Unlock()

This is based on the observation that [2] can only happen either at the beginning or after the all previous goroutines that execute [2] is holding on c.Wait, due to the locker they shared. So subWG.Wait(), meaning that 2 (or number of the subscriptions) [2] is executed, it is only possible that one goroutine is not holding on c.Wait, which can be solved by asking for the locker to Lock another time.

Playground: https://play.golang.org/p/6mjUEcn3ec5

Upvotes: 1

LeGEC
LeGEC

Reputation: 51810

With the wg waitgroup (as coded in your current group) : when the subscribe function returns, you know that the waiting goroutine has at least started its execution.

So when your main function reaches button.Clicked.Broadcast(), there's a good chance the two goroutines are actually waiting on their button.Clicked.Wait() call.

Without the wg, you have no guarantee that the goroutines have even started, and your code may call button.Clicked.Broadcast() too soon.

Note that your use of wg merely makes it less probable for the deadlock to happen, but it won't prevent it in all cases.

Try compiling your binary with -race, and run it in a loop (e.g from bash : for i in {1..100}; do ./myprogram; done), I think you will see that the same problem happens sometimes.

Upvotes: 1

Related Questions