Browse Source

stream: fix destroy(err, cb) regression

Fixed a regression that caused the callback passed to destroy()
to not be called if the stream was already destroyed.
This caused a regression on the ws module in CITGM introduced by
https://github.com/nodejs/node/pull/12925.

PR-URL: https://github.com/nodejs/node/pull/13156
Fixes: https://github.com/websockets/ws/issues/1118
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Calvin Metcalf <calvin.metcalf@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
v6
Matteo Collina 8 years ago
parent
commit
ccd3eadbd7
  1. 5
      lib/internal/streams/destroy.js
  2. 22
      test/parallel/test-net-socket-destroy-send.js
  3. 14
      test/parallel/test-stream-readable-destroy.js
  4. 15
      test/parallel/test-stream-writable-destroy.js

5
lib/internal/streams/destroy.js

@ -8,7 +8,10 @@ function destroy(err, cb) {
this._writableState.destroyed; this._writableState.destroyed;
if (readableDestroyed || writableDestroyed) { if (readableDestroyed || writableDestroyed) {
if (err && (!this._writableState || !this._writableState.errorEmitted)) { if (cb) {
cb(err);
} else if (err &&
(!this._writableState || !this._writableState.errorEmitted)) {
process.nextTick(emitErrorNT, this, err); process.nextTick(emitErrorNT, this, err);
} }
return; return;

22
test/parallel/test-net-socket-destroy-send.js

@ -0,0 +1,22 @@
'use strict';
const common = require('../common');
const net = require('net');
const assert = require('assert');
const server = net.createServer();
server.listen(0, common.mustCall(function() {
const port = server.address().port;
const conn = net.createConnection(port);
conn.on('connect', common.mustCall(function() {
conn.destroy();
conn.on('error', common.mustCall(function(err) {
assert.strictEqual(err.message, 'This socket is closed');
}));
conn.write(Buffer.from('kaboom'), common.mustCall(function(err) {
assert.strictEqual(err.message, 'This socket is closed');
}));
server.close();
}));
}));

14
test/parallel/test-stream-readable-destroy.js

@ -160,3 +160,17 @@ const { inherits } = require('util');
new MyReadable(); new MyReadable();
} }
{
// destroy and destroy callback
const read = new Readable({
read() {}
});
read.resume();
const expected = new Error('kaboom');
read.destroy(expected, common.mustCall(function(err) {
assert.strictEqual(expected, err);
}));
}

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

@ -170,3 +170,18 @@ const { inherits } = require('util');
new MyWritable(); new MyWritable();
} }
{
// destroy and destroy callback
const write = new Writable({
write(chunk, enc, cb) { cb(); }
});
write.destroy();
const expected = new Error('kaboom');
write.destroy(expected, common.mustCall(function(err) {
assert.strictEqual(expected, err);
}));
}

Loading…
Cancel
Save