Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 6ce4ef3

Browse filesBrowse files
addaleaxMylesBorins
authored andcommitted
stream: make .destroy() interact better with write queue
Make sure that it is safe to call the callback for `_write()` even in the presence of `.destroy()` calls during that write. In particular, letting the write queue continue processing would previously have thrown an exception, because processing writes after calling `.destroy()` is forbidden. One test had to be modified to account for the fact that callbacks for writes will now always be called, even when the stream is destroyed during the process. PR-URL: #24062 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 897114b commit 6ce4ef3
Copy full SHA for 6ce4ef3

File tree

Expand file treeCollapse file tree

2 files changed

+60
-1
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+60
-1
lines changed
Open diff view settings
Collapse file

‎lib/_stream_writable.js‎

Copy file name to clipboardExpand all lines: lib/_stream_writable.js
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -456,7 +456,7 @@ function onwrite(stream, er) {
456456
onwriteError(stream, state, sync, er, cb);
457457
else {
458458
// Check if we're actually ready to finish, but don't emit yet
459-
var finished = needFinish(state);
459+
var finished = needFinish(state) || stream.destroyed;
460460

461461
if (!finished &&
462462
!state.corked &&
Collapse file
+59Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
'use strict';
2+
require('../common');
3+
const assert = require('assert');
4+
const { Writable } = require('stream');
5+
6+
// Test interaction between calling .destroy() on a writable and pending
7+
// writes.
8+
9+
for (const withPendingData of [ false, true ]) {
10+
for (const useEnd of [ false, true ]) {
11+
const callbacks = [];
12+
13+
const w = new Writable({
14+
write(data, enc, cb) {
15+
callbacks.push(cb);
16+
},
17+
// Effectively disable the HWM to observe 'drain' events more easily.
18+
highWaterMark: 1
19+
});
20+
21+
let chunksWritten = 0;
22+
let drains = 0;
23+
let finished = false;
24+
w.on('drain', () => drains++);
25+
w.on('finish', () => finished = true);
26+
27+
w.write('abc', () => chunksWritten++);
28+
assert.strictEqual(chunksWritten, 0);
29+
assert.strictEqual(drains, 0);
30+
callbacks.shift()();
31+
assert.strictEqual(chunksWritten, 1);
32+
assert.strictEqual(drains, 1);
33+
34+
if (withPendingData) {
35+
// Test 2 cases: There either is or is not data still in the write queue.
36+
// (The second write will never actually get executed either way.)
37+
w.write('def', () => chunksWritten++);
38+
}
39+
if (useEnd) {
40+
// Again, test 2 cases: Either we indicate that we want to end the
41+
// writable or not.
42+
w.end('ghi', () => chunksWritten++);
43+
} else {
44+
w.write('ghi', () => chunksWritten++);
45+
}
46+
47+
assert.strictEqual(chunksWritten, 1);
48+
w.destroy();
49+
assert.strictEqual(chunksWritten, 1);
50+
callbacks.shift()();
51+
assert.strictEqual(chunksWritten, 2);
52+
assert.strictEqual(callbacks.length, 0);
53+
assert.strictEqual(drains, 1);
54+
55+
// When we used `.end()`, we see the 'finished' event if and only if
56+
// we actually finished processing the write queue.
57+
assert.strictEqual(finished, !withPendingData && useEnd);
58+
}
59+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.