From a3602435ae0ed6a5eea049c965f21e2ce0818aa5 Mon Sep 17 00:00:00 2001 From: junyup0503 <97156898+junyup0503@users.noreply.github.com> Date: Fri, 16 Oct 2026 14:57:23 +0900 Subject: [PATCH 1/4] http: emit 'drain' after flushing coalesced chunks Since chunked writes are coalesced while the message is corked, write() decides backpressure from the byte length of the buffered chunks and sets kNeedDrain. When uncork() flushes them, net.Socket accounts string chunks in UTF-16 code units, so with multi-byte text it can stay below its own highWaterMark. It then never needs to drain and never emits 'drain'. If the flushed writes complete asynchronously, the check at the end of uncork() also runs while bytes are still pending, so the response stalls with writableNeedDrain === true and writableLength === 0. Re-check the drain condition once the flushed data has been written, unless the message or its socket has been destroyed in the meantime. Fixes: https://github.com/nodejs/node/issues/66781 Refs: https://github.com/nodejs/node/pull/64987 Assisted-by: AI coding agent Signed-off-by: junyup0503 <97156898+junyup0503@users.noreply.github.com> --- lib/internal/http/outgoing.js | 9 +++ ...t-http-outgoing-chunked-drain-multibyte.js | 72 +++++++++++++++++++ 2 files changed, 81 insertions(+) create mode 100644 test/parallel/test-http-outgoing-chunked-drain-multibyte.js diff --git a/lib/internal/http/outgoing.js b/lib/internal/http/outgoing.js index ad0ad6dc372c..ff03210f19e8 100644 --- a/lib/internal/http/outgoing.js +++ b/lib/internal/http/outgoing.js @@ -376,6 +376,15 @@ OutgoingMessage.prototype.uncork = function uncork() { for (const callback of callbacks) { callback(err); } + // write() returned false based on the byte length of the coalesced + // chunks, but net.Socket counts string chunks in UTF-16 code units and + // may stay below its own highWaterMark, so it never emits 'drain'. + // Re-check once the flushed data has been written. + if (!err && this.writableNeedDrain && !socket.destroyed && + this.writableLength === 0) { + this[kNeedDrain] = false; + this.emit('drain'); + } }); this[kChunkedBuffer].length = 0; diff --git a/test/parallel/test-http-outgoing-chunked-drain-multibyte.js b/test/parallel/test-http-outgoing-chunked-drain-multibyte.js new file mode 100644 index 000000000000..f3427a1f8477 --- /dev/null +++ b/test/parallel/test-http-outgoing-chunked-drain-multibyte.js @@ -0,0 +1,72 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); +const { Duplex } = require('stream'); + +// Refs: https://github.com/nodejs/node/issues/66781 +// A chunked response written with multi-byte string chunks must emit 'drain' +// after write() returned false. write() decides backpressure from the byte +// length of the auto-corked chunks, while net.Socket counts string chunks in +// UTF-16 code units, so the socket may stay below its own highWaterMark and +// never emit 'drain' itself once the coalesced chunks are flushed. + +// Behaves like net.Socket for this purpose: strings are counted in UTF-16 +// code units and writes complete asynchronously, so flushed data is still in +// flight when uncork() returns. +function createSocket() { + const pending = []; + const socket = new Duplex({ + decodeStrings: false, + read() {}, + write(chunk, encoding, callback) { + pending.push(callback); + }, + }); + // Complete one write per turn of the event loop. + (function flush() { + pending.shift()?.(); + if (!socket.destroyed) setImmediate(flush); + })(); + return socket; +} + +const row = 'a'.repeat(100) + 'ع'.repeat(150) + '\n'; // 401 bytes + +{ + const ROWS = 2000; + const socket = createSocket(); + const server = http.createServer(common.mustCall(async (req, res) => { + let drains = 0; + for (let i = 0; i < ROWS; i++) { + if (!res.write(row)) { + assert.strictEqual(res.writableNeedDrain, true); + await new Promise((resolve) => res.once('drain', resolve)); + assert.strictEqual(res.writableLength, 0); + assert.strictEqual(res.writableNeedDrain, false); + drains++; + } + } + assert.ok(drains > 0); + res.end(common.mustCall(() => socket.destroy())); + })); + + server.emit('connection', socket); + socket.push('GET / HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n'); +} + +// 'drain' must not be emitted once the response has been destroyed from a +// write callback. +{ + const socket = createSocket(); + const server = http.createServer(common.mustCall((req, res) => { + res.on('drain', common.mustNotCall()); + const ret = res.write(row.repeat(200), common.mustCall(() => { + res.destroy(); + })); + assert.strictEqual(ret, false); + })); + + server.emit('connection', socket); + socket.push('GET / HTTP/1.1\r\nHost: localhost\r\n\r\n'); +} From 96a037750f340c4ca4de77b7f11b6d50941de21d Mon Sep 17 00:00:00 2001 From: junyup0503 <97156898+junyup0503@users.noreply.github.com> Date: Fri, 30 Oct 2026 03:22:53 +0900 Subject: [PATCH 2/4] http: recheck current socket before emitting drain Read the current socket after the coalesced write callbacks run, since uncork() may initially buffer data before a socket is assigned. Check that the current socket exists and is not destroyed before emitting drain. Refs: https://github.com/nodejs/node/pull/66782#discussion_r4291836598 Assisted-by: AI coding agent Signed-off-by: junyup0503 <97156898+junyup0503@users.noreply.github.com> --- lib/internal/http/outgoing.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/internal/http/outgoing.js b/lib/internal/http/outgoing.js index ff03210f19e8..92a9c1eafff0 100644 --- a/lib/internal/http/outgoing.js +++ b/lib/internal/http/outgoing.js @@ -380,8 +380,9 @@ OutgoingMessage.prototype.uncork = function uncork() { // chunks, but net.Socket counts string chunks in UTF-16 code units and // may stay below its own highWaterMark, so it never emits 'drain'. // Re-check once the flushed data has been written. - if (!err && this.writableNeedDrain && !socket.destroyed && - this.writableLength === 0) { + const currentSocket = this[kSocket]; + if (!err && this.writableNeedDrain && currentSocket && + !currentSocket.destroyed && this.writableLength === 0) { this[kNeedDrain] = false; this.emit('drain'); } From 3f263a824f46b000e99da065c3ac1f53091cce76 Mon Sep 17 00:00:00 2001 From: junyup0503 <97156898+junyup0503@users.noreply.github.com> Date: Tue, 3 Nov 2026 14:39:52 +0900 Subject: [PATCH 3/4] http: check message state before emitting drain Rely on the message's own destroyed state instead of the socket's when re-checking the drain condition after the coalesced chunks are flushed. writableNeedDrain already excludes destroyed and finished messages. Refs: https://github.com/nodejs/node/pull/66782#discussion_r4435933658 Co-authored-by: Robert Nagy Co-authored-by: Park Jin Woong <35551417+par333k@users.noreply.github.com> Assisted-by: AI coding agent Signed-off-by: junyup0503 <97156898+junyup0503@users.noreply.github.com> --- lib/internal/http/outgoing.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/internal/http/outgoing.js b/lib/internal/http/outgoing.js index 92a9c1eafff0..92a325489f23 100644 --- a/lib/internal/http/outgoing.js +++ b/lib/internal/http/outgoing.js @@ -380,9 +380,7 @@ OutgoingMessage.prototype.uncork = function uncork() { // chunks, but net.Socket counts string chunks in UTF-16 code units and // may stay below its own highWaterMark, so it never emits 'drain'. // Re-check once the flushed data has been written. - const currentSocket = this[kSocket]; - if (!err && this.writableNeedDrain && currentSocket && - !currentSocket.destroyed && this.writableLength === 0) { + if (!err && this.writableNeedDrain && !this.destroyed && this.writableLength === 0) { this[kNeedDrain] = false; this.emit('drain'); } From 0ecaa638c93e163ce98aec5ba7ecfeb6d34288d7 Mon Sep 17 00:00:00 2001 From: junyup0503 <97156898+junyup0503@users.noreply.github.com> Date: Wed, 4 Nov 2026 12:57:23 +0900 Subject: [PATCH 4/4] test: pin socket highWaterMark in chunked drain test The default stream highWaterMark is 16 KiB on Windows and 64 KiB elsewhere. With the smaller value the fake socket exceeds its own highWaterMark in UTF-16 code units and emits 'drain' by itself before the write callback destroys the response, which trips mustNotCall(). Pin the highWaterMark so the payload sizes stay on the intended side of it on every platform, and assert that the socket never drains on its own in the destroy scenario. Refs: https://github.com/nodejs/node/pull/66782#issuecomment-6616926003 Assisted-by: AI coding agent Signed-off-by: junyup0503 <97156898+junyup0503@users.noreply.github.com> --- .../test-http-outgoing-chunked-drain-multibyte.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-http-outgoing-chunked-drain-multibyte.js b/test/parallel/test-http-outgoing-chunked-drain-multibyte.js index f3427a1f8477..75b3e63a371a 100644 --- a/test/parallel/test-http-outgoing-chunked-drain-multibyte.js +++ b/test/parallel/test-http-outgoing-chunked-drain-multibyte.js @@ -13,11 +13,15 @@ const { Duplex } = require('stream'); // Behaves like net.Socket for this purpose: strings are counted in UTF-16 // code units and writes complete asynchronously, so flushed data is still in -// flight when uncork() returns. +// flight when uncork() returns. The high water mark is pinned because the +// default is platform dependent (16 KiB on Windows), and the coalesced +// payloads below must exceed it in bytes while staying under it in code +// units, so that the socket never emits 'drain' on its own. function createSocket() { const pending = []; const socket = new Duplex({ decodeStrings: false, + writableHighWaterMark: 64 * 1024, read() {}, write(chunk, encoding, callback) { pending.push(callback); @@ -56,9 +60,11 @@ const row = 'a'.repeat(100) + 'ع'.repeat(150) + '\n'; // 401 bytes } // 'drain' must not be emitted once the response has been destroyed from a -// write callback. +// write callback. The socket stays below its own high water mark, so the +// only candidate emitter is the message itself. { const socket = createSocket(); + socket.on('drain', common.mustNotCall()); const server = http.createServer(common.mustCall((req, res) => { res.on('drain', common.mustNotCall()); const ret = res.write(row.repeat(200), common.mustCall(() => {