FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

stream: don't emit finish after destroy · nodejs/node@48e7840 · GitHub

/ node Public

Commit 48e7840

Browse files
ronag
committed
stream: don't emit finish after destroy
PR-URL: #40852 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent f1658bd commit 48e7840

8 files changed

Lines changed: 25 additions & 26 deletions

‎lib/internal/streams/writable.js‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -650,6 +650,7 @@ Writable.prototype.end = function(chunk, encoding, cb) {
650650

651651
function needFinish(state) {
652652
return (state.ending &&
653+
!state.destroyed &&
653654
state.constructed &&
654655
state.length === 0 &&
655656
!state.errored &&
@@ -732,11 +733,18 @@ function prefinish(stream, state) {
732733
function finishMaybe(stream, state, sync) {
733734
if (needFinish(state)) {
734735
prefinish(stream, state);
735-
if (state.pendingcb === 0 && needFinish(state)) {
736-
state.pendingcb++;
736+
if (state.pendingcb === 0) {
737737
if (sync) {
738-
process.nextTick(finish, stream, state);
739-
} else {
738+
state.pendingcb++;
739+
process.nextTick((stream, state) => {
740+
if (needFinish(state)) {
741+
finish(stream, state);
742+
} else {
743+
state.pendingcb--;
744+
}
745+
}, stream, state);
746+
} else if (needFinish(state)) {
747+
state.pendingcb++;
740748
finish(stream, state);
741749
}
742750
}

‎test/parallel/test-http2-client-upload.js‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ fs.readFile(loc, common.mustSucceed((data) => {
2222
const server = http2.createServer();
2323
let client;
2424

25-
const countdown = new Countdown(3, () => {
25+
const countdown = new Countdown(2, () => {
2626
server.close();
2727
client.close();
2828
});
@@ -50,7 +50,6 @@ fs.readFile(loc, common.mustSucceed((data) => {
5050
req.resume();
5151
req.on('end', common.mustCall());
5252

53-
req.on('finish', () => countdown.dec());
5453
const str = fs.createReadStream(loc);
5554
str.on('end', common.mustCall());
5655
str.on('close', () => countdown.dec());

‎test/parallel/test-http2-compat-socket-set.js‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -73,16 +73,6 @@ server.on('request', common.mustCall(function(request, response) {
7373
assert.throws(() => request.socket.pause = noop, errMsg);
7474
assert.throws(() => request.socket.resume = noop, errMsg);
7575

76-
request.stream.on('finish', common.mustCall(() => {
77-
setImmediate(() => {
78-
request.socket.setTimeout = noop;
79-
assert.strictEqual(request.stream.setTimeout, noop);
80-
81-
assert.strictEqual(request.stream._isProcessing, undefined);
82-
request.socket._isProcessing = true;
83-
assert.strictEqual(request.stream._isProcessing, true);
84-
});
85-
}));
8676
response.stream.destroy();
8777
}));
8878

‎test/parallel/test-stream-duplex-destroy.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ const assert = require('assert');
125125
duplex.removeListener('end', fail);
126126
duplex.removeListener('finish', fail);
127127
duplex.on('end', common.mustNotCall());
128-
duplex.on('finish', common.mustCall());
128+
duplex.on('finish', common.mustNotCall());
129129
assert.strictEqual(duplex.destroyed, true);
130130
}
131131

‎test/parallel/test-stream-transform-destroy.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ const assert = require('assert');
117117
transform.removeListener('end', fail);
118118
transform.removeListener('finish', fail);
119119
transform.on('end', common.mustCall());
120-
transform.on('finish', common.mustCall());
120+
transform.on('finish', common.mustNotCall());
121121
}
122122

123123
{

‎test/parallel/test-stream-writable-destroy.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,6 @@ const assert = require('assert');
124124

125125
write.destroy();
126126

127-
write.removeListener('finish', fail);
128-
write.on('finish', common.mustCall());
129127
assert.strictEqual(write.destroyed, true);
130128
}
131129

‎test/parallel/test-stream-writable-finish-destroyed.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,13 @@ const { Writable } = require('stream');
3131
w.write('asd');
3232
w.destroy();
3333
}
34+
35+
{
36+
const w = new Writable({
37+
write() {
38+
}
39+
});
40+
w.on('finish', common.mustNotCall());
41+
w.end();
42+
w.destroy();
43+
}

‎test/parallel/test-stream-write-destroy.js‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,7 @@ for (const withPendingData of [ false, true ]) {
2020

2121
let chunksWritten = 0;
2222
let drains = 0;
23-
let finished = false;
2423
w.on('drain', () => drains++);
25-
w.on('finish', () => finished = true);
2624

2725
function onWrite(err) {
2826
if (err) {
@@ -60,9 +58,5 @@ for (const withPendingData of [ false, true ]) {
6058
assert.strictEqual(chunksWritten, useEnd && !withPendingData ? 1 : 2);
6159
assert.strictEqual(callbacks.length, 0);
6260
assert.strictEqual(drains, 1);
63-
64-
// When we used `.end()`, we see the 'finished' event if and only if
65-
// we actually finished processing the write queue.
66-
assert.strictEqual(finished, !withPendingData && useEnd);
6761
}
6862
}

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL