http: fix http agent keep alive

[theanarkh]

When options.keepAlive in createSocket function is true it leads to a bug. because createSocket will call this.createConnection(options, oncreate) which will create a socket and set two fields in socket.

this[kSetKeepAlive] = Boolean(options.keepAlive); // true
this[kSetKeepAliveInitialDelay] = ~~(options.keepAliveInitialDelay / 1000); // 0

this[kSetKeepAliveInitialDelay] will be 0 because options.keepAliveInitialDelay is undefined. When the connection is finished, afterConnect will be called and use this two fields, the related code is as follow.

if (self[kSetKeepAlive] && self._handle.setKeepAlive) {
      self._handle.setKeepAlive(true, self[kSetKeepAliveInitialDelay]);
}

It calls setKeepAlive with 0 (self[kSetKeepAliveInitialDelay]).

Then when the free event of agent is emitted, agent will call setKeepAlive in keepSocketAlive, the code is as follow.

// enable is true and this[kSetKeepAlive] is true too
if (this._handle.setKeepAlive && enable !== this[kSetKeepAlive]) {
    this[kSetKeepAlive] = enable;
    this[kSetKeepAliveInitialDelay] = initialDelay;
    this._handle.setKeepAlive(enable, initialDelay);
 }

enable !== this[kSetKeepAlive] will return false, so the agent do nothing which lead to a bug.

Currently http agent only set keepalive on some sockets (when free event is emitted). Maybe we can set keepalive for all sockets ? otherwise i think we should delete the keepAlive field of options before call this.createConnection in createSocket.

Refs: #41965.

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

Affected subsystem: http

[nodejs-github-bot]

Review requested:

• @nodejs/http
• @nodejs/net

[mscdex]

This needs a test

[ShogunPanda]

LGTM!

[mcollina]

lgtm

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/44492/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/44510/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/44519/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/44578/

[theanarkh]

@ShogunPanda Hi, can you help trigger CI again ? Thanks !

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/44621/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/44625/

[nodejs-github-bot]

Landed in fe776b8