Sometimes I see code where wg.Done() is not being called in the end of the goroutine

ferraro 0
Viewed 51

I am new to the Golang, please bear me and kindly clarify my query.

what I understood from wg.Done() call, this is an indication to WaitGroip that my goroutine is done so that wait call by WaitGroup will end, let's consider a scenario where we have only 1 goroutine running, So generally we use defer keyword for wg.Done() so it will get called at the end of the goroutine block.

I came across below code snippet, where wg.Done() it's being called in the midway, I am confused why it's being called here and also when I practiced code using wg.Done() in the midway, code after that it's not been called, So with that in mind I am confused in the below codebase wg.Done() is being called in the mid-way.

func startNetworkDaemon() *sync.WaitGroup {
    var wg sync.WaitGroup
    wg.Add(1)
    go func() {
        connPool := warmServiceConnCache()

        server, err := net.Listen("tcp", "localhost:8080")
        if err != nil {
            log.Fatalf("cannot listen: %v", err)
        }
        defer server.Close()

        wg.Done()

        for {
            conn, err := server.Accept()
            if err != nil {
                log.Printf("cannot accept connection: %v", err)
                continue
            }
            svcConn := connPool.Get()
            fmt.Fprintln(conn, "")
            connPool.Put(svcConn)
            conn.Close()
        }
    }()
    return &wg
}

Another code sample as well, in the below code if add defer keyword to the wg.Done() method, I am getting a deadlock error. Can someone explain the reason for this...

func usingBroadcast() {

  beeper := sync.NewCond(&sync.Mutex{})
  var wg sync.WaitGroup
  wg.Add(1)

  miniFunc(func() {
    fmt.Println("mini1")  
    wg.Done() 
  }, beeper) 

  beeper.Broadcast() 
  wg.Wait()
  
}

func miniFunc(fn func(), beeper *sync.Cond) {

    var wg sync.WaitGroup
    wg.Add(1)
    go func() {
      wg.Done()
      beeper.L.Lock()
      fmt.Println("i am waiting")
      beeper.Wait()
      beeper.L.Unlock()
      fn()
      
    }()

    wg.Wait()
}
go
3 Answers

A waitgroup waits until all that are added into the group are done. Those things in the waitgroup can be goroutines, or something else. In the first code snippet, it looks like the waitgroup is there to capture the state where the goroutine completed initialization. Once the goroutine calls the net.Listen, it notifies the waiting goroutine that the initialization is complete.

In the second example, there is a potential wait, and the waitgroup is notified before the wait. It looks like a scheme to make sure that before the waitgroup is notified the goroutine started running.

Burak Serdar0

The wait group is not needed in the examples.

If you look at the caller of startNetworkDaemon, you will find that it immediately calls Wait on the returned wait group. The caller is basically waiting on the creation on of the listener. Here's a refactoring that accomplishes the same goal:

func startNetworkDaemon() {
    connPool := warmServiceConnCache()

    server, err := net.Listen("tcp", "localhost:8080")
    if err != nil {
        log.Fatalf("cannot listen: %v", err)
    }

    go func() {
        defer server.Close()
        for {
            conn, err := server.Accept()
            if err != nil {
                log.Printf("cannot accept connection: %v", err)
                continue
            }
            svcConn := connPool.Get()
            fmt.Fprintln(conn, "")
            connPool.Put(svcConn)
            conn.Close()
        }
    }()
}

Because goroutine does not do anything before calling Done in the miniFunc example, the wait group serves no purpose. Refactor by removing the wait group.

func miniFunc(fn func(), beeper *sync.Cond) {
    go func() {
      beeper.L.Lock()
      fmt.Println("i am waiting")
      beeper.Wait()
      beeper.L.Unlock()
      fn()
    }()
}
memcpy0

(not a direct answer to the question, more of a formatted comment)

The second example shows an incorrect usage of sync operations. the shown example may deadlock (click several times on the [Run] button).

One way to avoid deadlock is to move the wg.Wait() operation in miniFunc, and have a way for the external caller to make sure that miniFunc() is in .Waiting state as in this snippet :

func usingBroadcast() {

    beeper := sync.NewCond(&sync.Mutex{})
    var wg sync.WaitGroup
    wg.Add(1)

    miniFunc(func() {
        fmt.Println("mini1")
        wg.Done()
    }, beeper)

    beeper.L.Lock() // call Lock / Unlock to make sure that the goroutine is in the '.Wait'ing state
    beeper.L.Unlock()

    beeper.Broadcast()
    wg.Wait()

}

func miniFunc(fn func(), beeper *sync.Cond) {

    var wg sync.WaitGroup
    wg.Add(1)
    go func() {
        beeper.L.Lock()
        wg.Done() // move wg.Done() after beeper.L.Lock()
        fmt.Println("i am waiting")
        beeper.Wait()
        beeper.L.Unlock()
        fn()

    }()

    wg.Wait()
}
LeGEC0
Related