mirror of
https://github.com/nodejs/node.git
synced 2026-10-28 10:15:07 +08:00
Naman Trivedi
0700e749eb
When OutgoingMessage transitions from pre-socket buffering (Path B) to socket-connected writing (Path A), the backpressure domain changes — subsequent writes go directly to the socket, which enforces its own backpressure via socket.write() return values. The OM should emit drain at this transition point to signal that its buffer is clear and the caller can resume writing under the socket backpressure regime. Previously, _flush() gated drain emission on writableLength === 0 which included socket.writableLength. This conflated two independent backpressure domains: the OM pre-socket buffer and the socket kernel write queue. When the socket had a higher writableHighWaterMark than the OM (e.g. agent-reused socket from a prior request), the socket was never backpressured and never emitted drain, causing a permanent deadlock. Additionally, avoid reusing a pooled socket in http.Agent when its writableHighWaterMark differs from the request highWaterMark, so that the user backpressure threshold is respected for the common case of the built-in Agent. Signed-off-by: Naman Trivedi <trivenay@amazon.com> Fixes: https://github.com/nodejs/node/issues/64680 Refs: https://github.com/nodejs/node/pull/64653 Refs: https://github.com/nodejs/node/pull/62936 PR-URL: https://github.com/nodejs/node/pull/64991 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
58 lines
1.9 KiB
JavaScript
58 lines
1.9 KiB
JavaScript
'use strict';
|
|
|
|
// Regression test: when _flush() hands buffered data to a socket whose
|
|
// writableHighWaterMark is higher than the OutgoingMessage's kHighWaterMark,
|
|
// drain must still fire. Previously, _flush() gated drain emission on
|
|
// writableLength === 0, which included socket.writableLength — but the
|
|
// socket was never backpressured (data < socket HWM), so drain never fired.
|
|
//
|
|
// See: https://github.com/nodejs/node/issues/64680
|
|
|
|
const common = require('../common');
|
|
const assert = require('assert');
|
|
const http = require('http');
|
|
|
|
// Server that delays reading to keep socket.writableLength > 0 during flush.
|
|
const server = http.createServer(common.mustCall((req, res) => {
|
|
setTimeout(() => {
|
|
req.resume();
|
|
req.on('end', () => res.end('ok'));
|
|
}, 500);
|
|
}, 2));
|
|
|
|
server.listen(0, common.mustCall(() => {
|
|
const port = server.address().port;
|
|
const agent = new http.Agent({ keepAlive: true });
|
|
|
|
// Request A: creates socket with HWM=2MB.
|
|
http.request({
|
|
host: 'localhost', port, method: 'POST', agent,
|
|
highWaterMark: 2 * 1024 * 1024,
|
|
}, common.mustCall((res) => {
|
|
res.resume();
|
|
res.on('end', common.mustCall(() => {
|
|
// Wait for socket to return to pool.
|
|
setTimeout(common.mustCall(() => {
|
|
// Request B: default HWM (64KB), reuses socket (HWM=2MB).
|
|
// Write 500KB: above OM HWM (64KB), below socket HWM (2MB).
|
|
const reqB = http.request({
|
|
host: 'localhost', port, method: 'POST', agent,
|
|
}, common.mustCall((res2) => {
|
|
res2.resume();
|
|
res2.on('end', common.mustCall(() => {
|
|
server.close();
|
|
}));
|
|
}));
|
|
|
|
const result = reqB.write(Buffer.alloc(500 * 1024));
|
|
assert.strictEqual(result, false);
|
|
|
|
// Drain must fire — no deadlock.
|
|
reqB.on('drain', common.mustCall(() => {
|
|
reqB.end();
|
|
}));
|
|
}), 100);
|
|
}));
|
|
})).end('x');
|
|
}));
|