GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — http: emit drain on socket takeover and avoid stale HWM reuse · nodejs/node@dad9548

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
  • lib
    • _http_agent.js
    • _http_outgoing.js
  • test/parallel
    • test-http-agent-highwatermark-reuse.js
    • test-http-outgoing-flush-drain.js
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎lib/_http_agent.js‎

Copy file name to clipboardExpand all lines: lib/_http_agent.js
+10Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,16 @@ Agent.prototype.addRequest = function addRequest(req, options, port/* legacy */,
392392
const sockLen = freeLen + this.sockets[name].length;
393393

394394
// Reusing a socket from the pool.
395+
// If the caller specified a highWaterMark that differs from the pooled
396+
// socket's writableHighWaterMark, sync the socket's HWM so that
397+
// backpressure semantics match what the caller requested.
398+
if (socket && options.highWaterMark != null &&
399+
socket.writableHighWaterMark !== options.highWaterMark) {
400+
debug('sync reused socket HWM (socket=%d, request=%d)',
401+
socket.writableHighWaterMark, options.highWaterMark);
402+
socket._writableState.highWaterMark = options.highWaterMark;
403+
}
404+
395405
if (socket) {
396406
asyncResetHandle(socket);
397407
this.reuseSocket(socket, req);
Collapse file

‎lib/_http_outgoing.js‎

Copy file name to clipboardExpand all lines: lib/_http_outgoing.js
+4-2Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1230,12 +1230,14 @@ OutgoingMessage.prototype._flush = function _flush() {
12301230

12311231
if (socket?.writable) {
12321232
// There might be remaining data in this.output; write it out
1233-
this._flushOutput(socket);
1233+
const ret = this._flushOutput(socket);
12341234

12351235
if (this.finished) {
12361236
// This is a queue to the server or client to bring in the next this.
12371237
this._finish();
1238-
} else if (this[kNeedDrain] && this.writableLength === 0) {
1238+
} else if (this[kNeedDrain] && ret !== false) {
1239+
// Socket accepted all data without backpressure - it won't emit
1240+
// drain, so we emit it since the OM buffer is now clear.
12391241
this[kNeedDrain] = false;
12401242
this.emit('drain');
12411243
}
Collapse file

‎test/parallel/test-http-agent-highwatermark-reuse.js‎

Copy file name to clipboard
+56Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
'use strict';
2+
3+
// Regression test: when a pooled socket's writableHighWaterMark differs from
4+
// the new request's highWaterMark, the agent must sync the socket's HWM so
5+
// that backpressure semantics match what the caller requested.
6+
//
7+
// See: https://github.com/nodejs/node/issues/64680
8+
9+
const common = require('../common');
10+
const assert = require('assert');
11+
const http = require('http');
12+
13+
const server = http.createServer(common.mustCall((req, res) => {
14+
req.resume();
15+
req.on('end', () => res.end('ok'));
16+
}, 2));
17+
18+
server.listen(0, common.mustCall(() => {
19+
const port = server.address().port;
20+
const agent = new http.Agent({ keepAlive: true });
21+
22+
// Request A: creates socket with HWM=1MB.
23+
http.request({
24+
host: 'localhost', port, method: 'POST', agent,
25+
highWaterMark: 1024 * 1024,
26+
}, common.mustCall((res) => {
27+
res.resume();
28+
res.on('end', common.mustCall(() => {
29+
// Wait for socket to return to pool.
30+
setTimeout(common.mustCall(requestB), 100);
31+
}));
32+
})).end('x');
33+
34+
function requestB() {
35+
const freeCount = Object.values(agent.freeSockets).flat().length;
36+
assert.strictEqual(freeCount, 1);
37+
38+
// Request B: HWM=10KB — agent must sync the reused socket's HWM.
39+
const reqB = http.request({
40+
host: 'localhost', port, method: 'POST', agent,
41+
highWaterMark: 10 * 1024,
42+
}, common.mustCall((res) => {
43+
res.resume();
44+
res.on('end', common.mustCall(() => {
45+
server.close();
46+
}));
47+
}));
48+
49+
reqB.on('socket', common.mustCall((socket) => {
50+
// Socket HWM must be synced to the request's value.
51+
assert.strictEqual(socket.writableHighWaterMark, 10 * 1024);
52+
}));
53+
54+
reqB.end('y');
55+
}
56+
}));
Collapse file

‎test/parallel/test-http-outgoing-flush-drain.js‎

Copy file name to clipboard
+57Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
'use strict';
2+
3+
// Regression test: when _flush() hands buffered data to a socket whose
4+
// writableHighWaterMark is higher than the OutgoingMessage's kHighWaterMark,
5+
// drain must still fire. Previously, _flush() gated drain emission on
6+
// writableLength === 0, which included socket.writableLength — but the
7+
// socket was never backpressured (data < socket HWM), so drain never fired.
8+
//
9+
// See: https://github.com/nodejs/node/issues/64680
10+
11+
const common = require('../common');
12+
const assert = require('assert');
13+
const http = require('http');
14+
15+
// Server that delays reading to keep socket.writableLength > 0 during flush.
16+
const server = http.createServer(common.mustCall((req, res) => {
17+
setTimeout(() => {
18+
req.resume();
19+
req.on('end', () => res.end('ok'));
20+
}, 500);
21+
}, 2));
22+
23+
server.listen(0, common.mustCall(() => {
24+
const port = server.address().port;
25+
const agent = new http.Agent({ keepAlive: true });
26+
27+
// Request A: creates socket with HWM=2MB.
28+
http.request({
29+
host: 'localhost', port, method: 'POST', agent,
30+
highWaterMark: 2 * 1024 * 1024,
31+
}, common.mustCall((res) => {
32+
res.resume();
33+
res.on('end', common.mustCall(() => {
34+
// Wait for socket to return to pool.
35+
setTimeout(common.mustCall(() => {
36+
// Request B: default HWM (64KB), reuses socket (HWM=2MB).
37+
// Write 500KB: above OM HWM (64KB), below socket HWM (2MB).
38+
const reqB = http.request({
39+
host: 'localhost', port, method: 'POST', agent,
40+
}, common.mustCall((res2) => {
41+
res2.resume();
42+
res2.on('end', common.mustCall(() => {
43+
server.close();
44+
}));
45+
}));
46+
47+
const result = reqB.write(Buffer.alloc(500 * 1024));
48+
assert.strictEqual(result, false);
49+
50+
// Drain must fire — no deadlock.
51+
reqB.on('drain', common.mustCall(() => {
52+
reqB.end();
53+
}));
54+
}), 100);
55+
}));
56+
})).end('x');
57+
}));

0 commit comments

Comments
0 (0)
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.