Open asmpro opened 7 years ago
Forgot to write, that in my code OnIdle function calls p.Stop() after N idle calls.
Regards, Uros
I use RunLoop in the same way and p.Stop() don't stop the loop.
Here a little example:
p := fastping.NewPinger()
ra, err := net.ResolveIPAddr("ip4:icmp", "127.0.0.1")
if err != nil {
fmt.Println(err)
}
p.AddIPAddr(ra)
p.OnRecv = func(addr *net.IPAddr, rtt time.Duration) {
fmt.Println("receive")
p.Stop()
}
p.OnIdle = func() {
fmt.Println("idle")
p.Stop()
}
p.RunLoop()
ticker := time.NewTicker(time.Second * 2)
select {
case <-p.Done():
fmt.Println("loop done")
case <-ticker.C:
fmt.Println("timeout done")
break
}
ticker.Stop()
Stdout result is :
receive
timeout done
I just ping loopback with 2 sec of "timeout" and "loop done" was never printed.
I'm facing the same issue, but I see in mainloop this check:
mainloop:
for {
select {
case <-p.ctx.stop:
p.debugln("Run(): <-p.ctx.stop")
So shouldn't
func (p *Pinger) Stop()
return something like
<-p.ctx.stop
instead of <- p.ctx.done
?
cova-fe: I am almost sure the little bug is fixed by changing line 377 in Stop() function of fastping.go from:
<-p.ctx.done
to:
p.ctx.done <- true
You see, the problem in my opinion is in the fact, that Done() function in fact returns done channel, which must contain a boolean true value if you want to stop the loop.
I think that your loop should look similar to bewiwi's, so checking the p.Done() and not the p.ctx.stop itself.
This patch is tested and the code is running in production.
Regards, Uros
@asmpro Hi, I have a func here which has to run forever runloop and break whenever there is ping failure. The problem is it works fine till it can ping and goes to idle but doesn't stop or exit. Here is the code any help is appreciated. Thanks
func pingIP(ipadd string) { pinger := fastping.NewPinger() err := pinger.AddIP(ipadd) if err != nil { fmt.Printf("Error adding IP Address") panic(err) } pinger.OnRecv = func(addr net.IPAddr, rtt time.Duration) { fmt.Printf("IP Addr: %s receive, RTT: %v\n", addr.String(), rtt) } // pinger.MaxRTT = time.Second // ticker := time.NewTicker(time.Second 10) pinger.RunLoop() select { case <-pinger.Done(): if err := pinger.Err(); err != nil { fmt.Printf("Ping failed: %v", err) } // case <-ticker.C: // break } // ticker.Stop() pinger.Stop() os.Exit(1) }
@suvvari8 Hi,
First patch fastping.go as I mentioned in my first comment.
Then use RunLoop this way:
type PingData struct { PingReplies uint32 LastPingRTT time.Duration } type PingResult map[string]*PingData
p := fastping.NewPinger() pingRes := PingResult{} // Add ips p.AddIPAddr(ip) pingRes[ip] = &PingData{} // ... more ips (do not forget to set pingRes for each IP) // Set nping to number of pings per IP, you wish to process, for example 3 npings := 3 p.OnRecv = func(addr *net.IPAddr, rtt time.Duration) { addrStr := addr.String() ips[addrStr].PingReplies++ ips[addrStr].LastPingRTT = rtt }
pings := 0 p.OnIdle = func() { pings++ if pings >= npings { p.Stop() } }
p.RunLoop() <-p.Done() if err = p.Err(); err != nil { panic(err) }
I seem to have found the cause of deadlock. https://github.com/tatsushid/go-fastping/blob/master/fastping.go#L454
Since the code is executed in parallel, the operation time.NewTicker(p.MaxRTT) conflicts with "close (p.ctx.stop)" and when processing anything in
OnIdle = func () {
}
we get deadlock
Solution: do not use blocking operations in OnIdle
pings := 0
p.OnIdle = func() {
pings++
if pings >= npings {
- p.Stop()
+ go p.Stop()
}
}
or not use OnIdle
at all.
Hi, I noticed, that Stop() function does not write bool to done channel, but instead reads from it. In my code I am using the following:
which deadlocks, so I suggest Stop() function should be changed to:
func (p *Pinger) Stop() { p.debugln("Stop(): close(p.ctx.stop)") close(p.ctx.stop) p.debugln("Stop(): p.ctx.done <- true") p.ctx.done <- true }
Regards, Uros