Anyone using this library use this command

[davidrenne]

netstat -a | grep icmp | wc -l

This library we have implemented two different ways and have always found it keeping ICMP requests and not closing them somehow for some reason.

We rewrote our own pinger and it has no leaks. I suggest the author run all their tests using a long-running process like a web server calling fastping and ensuring that there are no ICMP requests growing with this command. Using a unit test which exits out quickly will not show how these ICMP requests possibly are still open.

There is definitely a possibility it could have been our code causing the leak, but we gave it a second chance and used channels and 100% knew it was not our library keeping these open, yet it still leaked ICMP counts which overtime will completely hose a server if there are too many being held open.

Hopefully, this command is helpful for anyone using this library (or the authors) to be wary of using this library and keeping watch on your ICMP counts bloating and taking down webservers.

I am not going to provide any code samples because we are not using this library anymore and I just wanted to leave that command with people just in case they read this and want to see if there are any leaked pings that can grow overtime and cause IRQ issues taking down a server and causing high load.

[kanocz]

I think you're right and there are leaks in original implementation + too high mutex usage which slowdown multi-thread pinging... I'm using modified version on high volume of pings without any problems

[davidrenne]

ill check out your fork @kanocz

[davidrenne]

@kanocz throw him a pull request. looks like you fixed a lot. Wish I had found your fork sooner or you got a pull request approved.

[kanocz]

@davidrenne I'm breaking API compatibility a lot, so pull request is not the best idea :-(

[davidrenne]

@kanocz ya probably, but I would take instability and a broken API vs. someone coming across and using this library without knowing this and taking down production machines because of this leak.

I would rather @tatsushid at least know about these issues and more people complaining about the leaks with concurrent usages and put them into the readme maybe explaining what you can and cannot do with it with known issues maybe.

Maybe if you know in detail what are the leaks and how to reproduce, maybe just a README update explaining there are known issues with concurrency and this library keeping ICMP's open and that they can use your library if they desire to use fastping in a concurrent manner. Its a pretty good step forward to either fix those issues in the main author's branch or explain them so this library doesnt tank a server. I dont think it is the authors intent to cause computer problems of those implementing callers, but someone should explain it as a known issue so its clear when using it to watch out for ICMP leaks.

I guess I am ranting because both this library and our own Lock issues caused two separate major issues and was a huge headache to gut out and fix both of them so I am ranting about the frustrations of using this library and just trying to get the voice out that something should be disclaimed about this issue so another person's day wasnt like mine. That shell script showed the leak until we got to maybe 1700 pings or so leaking, then the server took a dive on CPU.

Regardless I appreciate all the hardwork from you and the author of this, but as you guys know how frustrating things can be when a server is taking a shit and finding the problems can be time-consuming, so the only reason I am adding more keywords is for google matches when the next person using this libary is seeing high load, they will thank me for voicing these issues that are not in the README as limitations.

Back to "happy coding" for me.

[davidrenne]

Unfortunately we didn't use another library just called os exec and if one of the pings out of the count passed pingCount contains bytes from was good enough for us.

cmd := "/sbin/ping"
if runtime.GOOS == "linux" {
cmd = "/bin/ping"
}

out, err := exec.Command(cmd, "-c "+extensions.IntToString(pingCount), "-i 1", ipAddress).Output()
if err != nil {
d.Lock()
d.canPing = false
d.Unlock()
return
}

if strings.Contains(string(out), "bytes from") {

d.Lock()
d.canPing = true
d.Unlock()
return
} else {
d.Lock()
d.canPing = false
d.Unlock()
return
}