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

stream: do not flush destroyed writable · nodejs/node@aa32e13 · GitHub

/ node Public

Commit aa32e13

Browse files
ronag
authored and
Trott
committed
stream: do not flush destroyed writable
It doesn't make much sense to flush a stream which has been destroyed. PR-URL: #29028 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent 95d6ad6 commit aa32e13

4 files changed

Lines changed: 85 additions & 8 deletions

File tree

‎lib/_stream_writable.js‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -299,9 +299,13 @@ Writable.prototype.write = function(chunk, encoding, cb) {
299299
if (typeof cb !== 'function')
300300
cb = nop;
301301

302-
if (state.ending)
302+
if (state.ending) {
303303
writeAfterEnd(this, cb);
304-
else if (isBuf || validChunk(this, state, chunk, cb)) {
304+
} else if (state.destroyed) {
305+
const err = new ERR_STREAM_DESTROYED('write');
306+
process.nextTick(cb, err);
307+
errorOrDestroy(this, err);
308+
} else if (isBuf || validChunk(this, state, chunk, cb)) {
305309
state.pendingcb++;
306310
ret = writeOrBuffer(this, state, isBuf, chunk, encoding, cb);
307311
}
@@ -733,7 +737,21 @@ Object.defineProperty(Writable.prototype, 'writableFinished', {
733737
}
734738
});
735739

736-
Writable.prototype.destroy = destroyImpl.destroy;
740+
const destroy = destroyImpl.destroy;
741+
Writable.prototype.destroy = function(err, cb) {
742+
const state = this._writableState;
743+
if (!state.destroyed) {
744+
for (let entry = state.bufferedRequest; entry; entry = entry.next) {
745+
process.nextTick(entry.callback, new ERR_STREAM_DESTROYED('write'));
746+
}
747+
state.bufferedRequest = null;
748+
state.lastBufferedRequest = null;
749+
state.bufferedRequestCount = 0;
750+
}
751+
destroy.call(this, err, cb);
752+
return this;
753+
};
754+
737755
Writable.prototype._undestroy = destroyImpl.undestroy;
738756
Writable.prototype._destroy = function(err, cb) {
739757
cb(err);

‎test/parallel/test-http2-server-stream-session-destroy.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,11 @@ server.on('stream', common.mustCall((stream) => {
3939
code: 'ERR_STREAM_WRITE_AFTER_END',
4040
message: 'write after end'
4141
}));
42-
assert.strictEqual(stream.write('data'), false);
42+
assert.strictEqual(stream.write('data', common.expectsError({
43+
type: Error,
44+
code: 'ERR_STREAM_WRITE_AFTER_END',
45+
message: 'write after end'
46+
})), false);
4347
}));
4448

4549
server.listen(0, common.mustCall(() => {

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,3 +232,49 @@ const assert = require('assert');
232232
write._undestroy();
233233
write.end();
234234
}
235+
236+
{
237+
const write = new Writable();
238+
239+
write.destroy();
240+
write.on('error', common.expectsError({
241+
type: Error,
242+
code: 'ERR_STREAM_DESTROYED',
243+
message: 'Cannot call write after a stream was destroyed'
244+
}));
245+
write.write('asd', common.expectsError({
246+
type: Error,
247+
code: 'ERR_STREAM_DESTROYED',
248+
message: 'Cannot call write after a stream was destroyed'
249+
}));
250+
}
251+
252+
{
253+
const write = new Writable({
254+
write(chunk, enc, cb) { cb(); }
255+
});
256+
257+
write.on('error', common.expectsError({
258+
type: Error,
259+
code: 'ERR_STREAM_DESTROYED',
260+
message: 'Cannot call write after a stream was destroyed'
261+
}));
262+
263+
write.cork();
264+
write.write('asd', common.mustCall());
265+
write.uncork();
266+
267+
write.cork();
268+
write.write('asd', common.expectsError({
269+
type: Error,
270+
code: 'ERR_STREAM_DESTROYED',
271+
message: 'Cannot call write after a stream was destroyed'
272+
}));
273+
write.destroy();
274+
write.write('asd', common.expectsError({
275+
type: Error,
276+
code: 'ERR_STREAM_DESTROYED',
277+
message: 'Cannot call write after a stream was destroyed'
278+
}));
279+
write.uncork();
280+
}

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

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,16 @@ for (const withPendingData of [ false, true ]) {
2424
w.on('drain', () => drains++);
2525
w.on('finish', () => finished = true);
2626

27-
w.write('abc', () => chunksWritten++);
27+
function onWrite(err) {
28+
if (err) {
29+
assert.strictEqual(w.destroyed, true);
30+
assert.strictEqual(err.code, 'ERR_STREAM_DESTROYED');
31+
} else {
32+
chunksWritten++;
33+
}
34+
}
35+
36+
w.write('abc', onWrite);
2837
assert.strictEqual(chunksWritten, 0);
2938
assert.strictEqual(drains, 0);
3039
callbacks.shift()();
@@ -34,14 +43,14 @@ for (const withPendingData of [ false, true ]) {
3443
if (withPendingData) {
3544
// Test 2 cases: There either is or is not data still in the write queue.
3645
// (The second write will never actually get executed either way.)
37-
w.write('def', () => chunksWritten++);
46+
w.write('def', onWrite);
3847
}
3948
if (useEnd) {
4049
// Again, test 2 cases: Either we indicate that we want to end the
4150
// writable or not.
42-
w.end('ghi', () => chunksWritten++);
51+
w.end('ghi', onWrite);
4352
} else {
44-
w.write('ghi', () => chunksWritten++);
53+
w.write('ghi', onWrite);
4554
}
4655

4756
assert.strictEqual(chunksWritten, 1);

0 commit comments

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