This repository has been archived by the owner. It is now read-only.

Added interval and count options to setKeepAlive. #4882

wants to merge 1 commit into


None yet
6 participants

See libuv pull request: joyent/libuv#729

I tried to make a test for this to include it with the commit, but unfortunately it's a lot harder than I thought to trick Linux into not sending a FIN/RST, so I just tested it by unplugging an ethernet cable of another computer and watching netstat -to or TCPView on Windows.

There is a check in libuv to at least set the params and make sure setsockopt doesn't fail.

Added interval and count options to setKeepAlive.
The OS defaults for these are too extreme if you want to run a
chat server and know if someone disconnects. Changes go along
with libuv changes.

Can one of the admins verify this patch?

isaacs commented Mar 19, 2013

The change looks fine (in master, not v0.10) but it needs a test.

@isaacs I mentioned above that there is a test in libuv for the setsockopt, but making a test for the actual keep-alive working was something I couldn't do. The best I could do is to get the kernel to send a RST to the other end, but as soon as Node got the RST it closed the connection. I need to somehow simulate a case where the receiving end doesn't respond to keep-alive's but didn't send a FIN/RST. Any ideas of how I would do that cross-platform?

@tjfontaine tjfontaine modified the milestones: v0.13, v0.12 Feb 16, 2014

I'm going to move this out to the 0.13 milestone as it has yet to have been addressed in libuv either.

But here are my $0.02 on this API, we should be exposing something along the lines of something more direct for setsockopt. This and the libuv portion add some of those concepts but without a flexible API. Also I'd probably advocate further additional arguments to be in objects not positional.

@trevnorris trevnorris added timer and removed timer labels Feb 18, 2014


jasnell commented Aug 6, 2015

Given that (a) this is an API change that had been pushed out to v0.13 and (b) the PR needs updating before it can hope to land, I'm going to recommend closing this. If someone wishes to pursue this further, updating the PR and targeting it against either or would be the most appropriate. Closing for now but can reopen if there are objections. /cc

@jasnell jasnell closed this Aug 6, 2015

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.