go-ping / ping

ICMP Ping library for Go
MIT License
1.33k stars 345 forks source link

deal with deadlock issue #95

Closed jingor9394 closed 4 years ago

jingor9394 commented 4 years ago

A deadlock issue will come up in a high probability. That's because the recv channel is a buffered channel with only 5 buffers and the channel is full before it gets started. In the method p.recvICMP(), it keeps sending packet to the recv channel, and sometimes makes the channel full in a very short time. As you know, the goroutine will be blocked when you try to send items to a full channel. This issue always happens when you try to ping large amount of IPs.

I have noticed that some people have realized this issue. Some of them try to improve the buffers of the channel, but it can't guarantee that the channel will not be full again. Some of them try to give a short time break after sending packet to the recv channel, but it will influence the effects of the interval parameter.

Therefor the way to solve this problem is to let the recv channel get started before it receives packets. I create a goroutine before p.recvICMP() runs, and it guarantees that the recv channel will not be blocked again.

Hope it helps.

dbzyuzin commented 4 years ago

I have a simpler solution. You must use select when sending data to the recv channel here. it will look like this:

select {
case recv <- &packet{bytes: bytes, nbytes: n, ttl: ttl}:
case <- p.done:
    return
}
CHTJonas commented 4 years ago

Hi @brzstizc6. #85 actually solves this same issue much more succinctly so I think we'd prefer to merge that but thank you.