TCP keepalive delay ignored if set before socket is connected

[davepacheco]

TCP sockets have a setKeepAlive function that takes a boolean (for enabled) and an optional number (a number of milliseconds, which will be turned into seconds and then used to set TCP_KEEPIDLE). If you write a program like this:

var sock = require('net').connect({
    'host': '172.25.10.4',
    'port': 12345
});
sock.on('connect', function () {
        sock.setKeepAlive(true, 10000);
});

then this behaves as you'd expect. I'm testing this by setting up a server on the remote IP using:

$ nc -k -l -p 12345

and using this command to snoop traffic:

$ snoop -r -t a -d net1 port 12345

With the above program, snoop shows (output trimmed for clarity):

11:27:15.56483 172.25.10.105 -> 172.25.10.4  TCP D=12345 S=57053 Syn 
11:27:15.57475  172.25.10.4 -> 172.25.10.105 TCP D=57053 S=12345 Syn Ack
11:27:15.57478 172.25.10.105 -> 172.25.10.4  TCP D=12345 S=57053 Ack
11:27:25.58164 172.25.10.105 -> 172.25.10.4  TCP D=12345 S=57053 Push Ack
11:27:25.58168  172.25.10.4 -> 172.25.10.105 TCP D=57053 S=12345 Ack
11:27:35.59381 172.25.10.105 -> 172.25.10.4  TCP D=12345 S=57053 Push Ack
11:27:35.59386  172.25.10.4 -> 172.25.10.105 TCP D=57053 S=12345 Ack

That's the initial connection, followed by two rounds of keep-alive separated by 10 seconds each.

But if you change the program so that you call setKeepAlive right after calling connect(), but before the 'connected' event:

var sock = require('net').connect({
    'host': '172.25.10.4',
    'port': 12345
});
sock.setKeepAlive(true, 10000);

and run that, then snoop shows that we send keepalive packets every 60 seconds instead of every 10:

11:22:32.87803 172.25.10.105 -> 172.25.10.4  TCP D=12345 S=48739 Syn 
11:22:32.88793  172.25.10.4 -> 172.25.10.105 TCP D=48739 S=12345 Syn Ack 
11:22:32.88798 172.25.10.105 -> 172.25.10.4  TCP D=12345 S=48739 Ack 
11:23:32.89864 172.25.10.105 -> 172.25.10.4  TCP D=12345 S=48739 Push Ack 
11:23:32.89868  172.25.10.4 -> 172.25.10.105 TCP D=48739 S=12345 Ack 
11:24:32.91702 172.25.10.105 -> 172.25.10.4  TCP D=12345 S=48739 Push Ack 
11:24:32.91708  172.25.10.4 -> 172.25.10.105 TCP D=48739 S=12345 Ack 

Tracing this, I found that the code is pretty explicit about this. As you'd expect, we go through different code paths in the two cases. If you call setKeepAlive before being connected, then we set UV_TCP_KEEPALIVE on the libuv handle and apply it once we have an fd, but we always apply it with a delay of 60 seconds instead of whatever the user passed in:
https://github.com/joyent/node/blob/v0.12.2-release/deps/uv/src/unix/stream.c#L389-L391

That's because we never save the delay that the user passed in:
https://github.com/joyent/node/blob/v0.12.2-release/deps/uv/src/unix/tcp.c#L297-L310

Anyway, the net result of this is that the user asked for a delay of 10s, but Node silently picked a completely different value.

[misterdjules]

@davepacheco Thank you very much for the detailed bug report!

I would suggest fixing that in master since the behavior change could break a significant number of users, thus adding to the 0.13.1 milestone. @joyent/node-coreteam @davepacheco Thoughts?

[misterdjules]

Closing as a duplicate of #8572.

[davepacheco]

That seems reasonable.