Skip to content

Commit 85a223b

Browse files
davidje13aduh95
authored andcommitted
http: fix drain event with cork/uncork
When using cork() and uncork() with ServerResponse, the drain event was not reliably emitted after uncorking. This occurred because the uncork() method did not check if a drain was pending (kNeedDrain flag) after flushing the chunked buffer. This fix ensures that when uncork() successfully flushes buffered data and a drain was needed, the drain event is emitted immediately. This commit is a copy of PR #60437 (abandoned) with minor linting fixes. Fixes: #60432 Signed-off-by: David Evans <davidje13@users.noreply.github.com> PR-URL: #64038 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Tim Perry <pimterry@gmail.com>
1 parent fe674e9 commit 85a223b

2 files changed

Lines changed: 70 additions & 0 deletions

File tree

‎lib/_http_outgoing.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,12 @@ OutgoingMessage.prototype.uncork = function uncork() {
295295

296296
this[kChunkedBuffer].length=0;
297297
this[kChunkedLength]=0;
298+
299+
// If we had a pending drain and flushed all data, emit the drain event.
300+
if(this[kNeedDrain]&&this.writableLength===0){
301+
this[kNeedDrain]=false;
302+
this.emit('drain');
303+
}
298304
};
299305

300306
OutgoingMessage.prototype.setTimeout=functionsetTimeout(msecs,callback){
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
'use strict';
2+
constcommon=require('../common');
3+
consthttp=require('http');
4+
constassert=require('assert');
5+
6+
// Test that drain event is emitted correctly when using cork/uncork
7+
// with ServerResponse and the write buffer is full
8+
9+
constserver=http.createServer(common.mustCall(async(req,res)=>{
10+
res.cork();
11+
12+
// Write small amount - won't need drain
13+
assert.strictEqual(res.write('1'.repeat(10)),true);
14+
15+
// Write enough to exceed highWaterMark (set in 'connection' listener)
16+
assert.strictEqual(res.write('2'.repeat(1000)),false);
17+
18+
// Verify writableNeedDrain is set
19+
assert.strictEqual(res.writableNeedDrain,true);
20+
21+
// Wait for drain event after uncorking
22+
constdrainPromise=newPromise((resolve)=>{
23+
res.once('drain',common.mustCall(()=>{
24+
// After drain, writableNeedDrain should be false
25+
assert.strictEqual(res.writableNeedDrain,false);
26+
resolve();
27+
}));
28+
});
29+
30+
// Uncork should trigger drain
31+
res.uncork();
32+
awaitdrainPromise;
33+
34+
res.end();
35+
}));
36+
37+
server.on('connection',common.mustCall((socket)=>{
38+
// Set high water mark large enough to cover HTTP overhead + first
39+
// small content batch, but not enough to cover second batch.
40+
socket._writableState.highWaterMark=1000;
41+
}));
42+
43+
server.listen(0,common.localhostIPv4,common.mustCall(()=>{
44+
http.get({
45+
host: common.localhostIPv4,
46+
port: server.address().port,
47+
},common.mustCall((res)=>{
48+
letdata='';
49+
res.setEncoding('utf8');
50+
51+
res.on('data',(chunk)=>{
52+
data+=chunk;
53+
});
54+
55+
res.on('end',common.mustCall(()=>{
56+
// Verify we got all the data
57+
assert.strictEqual(data.length,10+1000);
58+
assert.strictEqual(data.substring(0,10),'1'.repeat(10));
59+
assert.strictEqual(data.substring(10),'2'.repeat(1000));
60+
61+
server.close(common.mustCall());
62+
}));
63+
}));
64+
}));

0 commit comments

Comments
 (0)