英文:
Best way of using sync.WaitGroup with external function
问题
我对以下代码有一些问题:
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() // 此处发生死锁
}
// 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 { // 从通道中读取,直到通道关闭
fmt.Println(n)
}
defer wg.Done()
}
我在指定的位置遇到了死锁。我尝试将wg.Add(1)
改为2,问题得到解决。我认为我没有成功将通道作为参数传递给Print
函数。有没有办法做到这一点?否则,解决我的问题的方法是将go Print(ch, wg)
行替换为:
go func() {
Print(ch)
defer wg.Done()
}
并将Print
函数更改为:
func Print(ch <-chan int) {
for n := range ch { // 从通道中读取,直到通道关闭
fmt.Println(n)
}
}
什么是最佳解决方案?
英文:
I have some issues 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's closed
fmt.Println(n)
}
defer wg.Done()
}
I get a deadlock at the specified place. I have tried setting wg.Add(1)
instead of 2 and it solves my problem. My belief is that I'm not successfully sending the channel as an argument to the Printer
function. Is there a way to do that? Otherwise, a solution to my problem is replacing the go Print(ch, wg)
line 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's closed
fmt.Println(n)
}
}
What is the best solution?
答案1
得分: 39
首先,你的实际错误是你给Print
方法传递了sync.WaitGroup
的一个副本,所以它不会在你等待的那个Wait()
方法上调用Done()
方法。
尝试使用以下代码替换:
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() // 此处发生死锁
}
func Print(ch <-chan int, wg *sync.WaitGroup) {
for n := range ch { // 从通道中读取数据,直到通道关闭
fmt.Println(n)
}
defer wg.Done()
}
现在,将你的Print
方法更改为不包含WaitGroup
是一个好主意:该方法不需要知道是否有其他地方在等待它完成工作。
英文:
Well, first your actual error is that you're giving the Print
method a copy of the sync.WaitGroup
, so it doesn't call the Done()
method on the one you're 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's closed
fmt.Println(n)
}
defer wg.Done()
}
Now, changing your Print
method to remove the WaitGroup
of it is a generally good idea: the method doesn't need to know something is waiting for it to finish its job.
答案2
得分: 4
我同意@Elwinar的解决方案,你的代码主要问题是将Waitgroup
的副本传递给了Print
函数。
这意味着在main
中定义的wg
的副本上执行了wg.Done()
。因此,main
中的wg
无法减少,当你在主函数中执行wg.Wait()
时就会发生死锁。
既然你也问到了最佳实践,我可以给你一些建议:
-
不要移除
Print
函数中的defer wg.Done()
。由于你的主goroutine是发送者,而print是接收者,移除接收者例程中的wg.Done()
会导致接收者未完成。这是因为只有发送者与主函数同步,所以在发送者完成后,主函数也完成了,但是接收者可能仍在工作。我的观点是:在主例程完成后,不要留下一些悬空的goroutine。关闭它们或等待它们。 -
记得在每个地方都进行panic恢复,特别是匿名goroutine。我见过很多golang程序员忘记在goroutine中放置panic恢复,即使他们记得在普通函数中放置recover。当你希望代码在发生意外情况时能够正确或至少优雅地运行时,这是至关重要的。
-
在每个关键调用之前使用
defer
,比如与sync
相关的调用,在代码的开头,因为你不知道代码可能在哪里出错。假设你在例子中的匿名goroutine中删除了wg.Done()
之前的defer
,并且发生了panic。如果你没有panic恢复,它将会panic。但是如果你有panic恢复会发生什么呢?现在一切都好了吗?不是的。由于panic的缘故,你的wg.Done()
被跳过了,所以在wg.Wait()
处会发生死锁!然而,通过使用defer
,即使发生panic,这个wg.Done()
也会在最后执行。同样,在close
之前使用defer也很重要,因为它的结果也会影响通信。
所以根据我上面提到的几点,这是修改后的代码:
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")
}
希望对你有帮助
英文:
I agree with @Elwinar's solution, that the main problem in your code caused by passing a copy of your Waitgroup
to the Print
function.
This means the wg.Done()
is operated on a copy of wg
you defined in the main
. Therefore, wg
in the main
could not get decreased, and thus a deadlock happens when you wg.Wait()
in main.
Since you are also asking about the best practice, I could give you some suggestions of my own:
-
Don't remove
defer wg.Done()
inPrint
. Since your goroutine in main is a sender, and print is a receiver, removingwg.Done()
in receiver routine will cause an unfinished receiver. This is because only your sender is synced with your main, so after your sender is done, your main is done, but it's possible that the receiver is still working. My point is: don't leave some dangling goroutines around after your main routine is finished. Close them or wait for them. -
Remember to do panic recovery everywhere, especially anonymous goroutine. I have seen a lot of golang programmers forgetting to put panic recovery in goroutines, even if they remember to put recover in normal functions. It's critical when you want your code to behave correctly or at least gracefully when something unexpected happened.
-
Use
defer
before every critical calls, likesync
related calls, at the beginning since you don't know where the code could break. Let's say you removeddefer
beforewg.Done()
, and a panic occurrs in your anonymous goroutine in your example. If you don't have panic recover, it will panic. But what happens if you have a panic recover? Everything's fine now? No. You will get deadlock atwg.Wait()
since yourwg.Done()
gets skipped because of panic! However, by usingdefer
, thiswg.Done()
will be executed at the end, even if panic happened. Also, defer beforeclose
is important too, since its result also affects the communication.
So here is the code modified according to the points I mentioned 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 it helps
通过集体智慧和协作来改善编程学习和解决问题的方式。致力于成为全球开发者共同参与的知识库,让每个人都能够通过互相帮助和分享经验来进步。
评论