Best way to use sync.WaitGroup with an external function

Question

I have some problems with the following code:

package main import ( "fmt" "sync" ) // This program should go to 11, but sometimes it only prints 1 to 10. func main() { ch := make(chan int) var wg sync.WaitGroup wg.Add(2) go Print(ch, wg) // go func(){ for i := 1; i <= 11; i++ { ch <- i } close(ch) defer wg.Done() }() wg.Wait() //deadlock here } // Print prints all numbers sent on the channel. // The function returns when the channel is closed. func Print(ch <-chan int, wg sync.WaitGroup) { for n := range ch { // reads from channel until it closed fmt.Println(n) } defer wg.Done() } 

I reached a dead end at the indicated location. I tried installing wg.Add(1) instead of 2, and it solves my problem. I am convinced that I am not sending a feed as an argument to the Printer function. Is there any way to do this? Otherwise, the solution to my problem replaces the go Print(ch, wg) with:

 go func() { Print(ch) defer wg.Done() } 

and changing the Printer function to:

 func Print(ch <-chan int) { for n := range ch { // reads from channel until it closed fmt.Println(n) } } 

What is the best solution?

Answer 1

Well, at first your actual mistake is that you give the Print method to the sync.WaitGroup copy, so it does not call the Done() method on the one you Wait() ing on.

Try this instead:

 package main import ( "fmt" "sync" ) func main() { ch := make(chan int) var wg sync.WaitGroup wg.Add(2) go Print(ch, &wg) go func() { for i := 1; i <= 11; i++ { ch <- i } close(ch) defer wg.Done() }() wg.Wait() //deadlock here } func Print(ch <-chan int, wg *sync.WaitGroup) { for n := range ch { // reads from channel until it closed fmt.Println(n) } defer wg.Done() } 

Now, by changing the Print method to remove WaitGroup , this is usually a good idea: the method should not know that something is waiting for it to complete.

Answer 2

I agree with @Elwinar's decision that the main problem in your code is caused by passing your Waitgroup function to Print .

This means that wg.Done() works with the copy of wg you defined in main . Therefore, wg in main cannot be reduced, and thus a dead end occurs when you wg.Wait() basically.

Since you are also asking about best practices, I can give you some suggestions:

• Do not remove defer wg.Done() in Print . Since your goroutine is mainly the sender and print is the receiver, removing wg.Done() in the receiver procedure will result in an incomplete receiver. This is because only your sender is synchronized with your main one, so after your sender is executed, your main function will be executed, but it is possible that the receiver is still working. My point: do not leave some dangling goroutines around after completing your main procedure. Close them or wait for them.

• Remember to damage the panic, especially the anonymous goroutine. I saw how many Golan programmers forget to panic in goroutines, even if they remember, to restore recovery to normal functions. This is important when you want your code to act correctly, or at least elegantly, when something unexpected happens.

• Use defer before each critical call, for example sync related calls, at the beginning , since you do not know where the code might break. Say you removed defer to wg.Done() , and in your example there was a panic in your anonymous goroutine. If you do not have a panic, she will panic. But what happens if you recover? Is everything alright now? No. You will get a dead end in wg.Wait() , since your wg.Done() will be skipped due to panic! However, using defer , this wg.Done() will be executed at the end, even if panic occurs. In addition, delaying to close also important, as its result also affects communication.

So, here is the code modified according to the points above:

 package main import ( "fmt" "sync" ) func main() { ch := make(chan int) var wg sync.WaitGroup wg.Add(2) go Print(ch, &wg) go func() { defer func() { if r := recover(); r != nil { println("panic:" + r.(string)) } }() defer func() { wg.Done() }() for i := 1; i <= 11; i++ { ch <- i if i == 7 { panic("ahaha") } } println("sender done") close(ch) }() wg.Wait() } func Print(ch <-chan int, wg *sync.WaitGroup) { defer func() { if r := recover(); r != nil { println("panic:" + r.(string)) } }() defer wg.Done() for n := range ch { fmt.Println(n) } println("print done") } 

Hope this helps :)