Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stream: add closed property #33990

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
stream: add closed property
fs streams already have a closed property. This PR standardized
it and brings it inline with rest of streams.
  • Loading branch information
ronag committed Jun 20, 2020
commit 1dafce82b91e8c8c7b164100898a3d10aed31d21
20 changes: 20 additions & 0 deletions doc/api/stream.md
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,16 @@ added: v8.0.0

Is `true` after [`writable.destroy()`][writable-destroy] has been called.

##### `writable.closed`
<!-- YAML
added: REPLACEME
-->

* {boolean}

Is `true` after `callback` in [`writable._destroy(err, callback)`][writable-_destroy]
ronag marked this conversation as resolved.
Show resolved Hide resolved
has been called.

##### `writable.end([chunk[, encoding]][, callback])`
<!-- YAML
added: v0.9.4
Expand Down Expand Up @@ -983,6 +993,16 @@ added: v8.0.0

Is `true` after [`readable.destroy()`][readable-destroy] has been called.

##### `readable.closed`
<!-- YAML
added: REPLACEME
-->

* {boolean}

Is `true` after `callback` in [`readable._destroy(err, callback)`][readable-_destroy]
ronag marked this conversation as resolved.
Show resolved Hide resolved
has been called.

##### `readable.isPaused()`
<!-- YAML
added: v0.11.14
Expand Down
18 changes: 18 additions & 0 deletions lib/_stream_duplex.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,5 +104,23 @@ ObjectDefineProperties(Duplex.prototype, {
this._writableState.destroyed = value;
}
}
},

closed: {
get() {
if (this._readableState === undefined ||
this._writableState === undefined) {
return false;
}
return this._readableState.closed && this._writableState.closed;
},
set(value) {
// Backward compatibility, the user is explicitly
// managing closed.
if (this._readableState && this._writableState) {
this._readableState.closed = value;
this._writableState.closed = value;
}
}
}
});
12 changes: 12 additions & 0 deletions lib/_stream_readable.js
Original file line number Diff line number Diff line change
Expand Up @@ -1167,6 +1167,18 @@ ObjectDefineProperties(Readable.prototype, {
}
},

closed: {
get() {
return this._readableState ? this._readableState.closed : false;
},
set(value) {
// Backward compatibility, the user is explicitly managing closed.
if (this._readableState) {
this._readableState.closed = value;
}
}
},

readableEnded: {
enumerable: false,
get() {
Expand Down
12 changes: 12 additions & 0 deletions lib/_stream_writable.js
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,18 @@ ObjectDefineProperties(Writable.prototype, {
}
},

closed: {
get() {
return this._writableState ? this._writableState.closed : false;
},
set(value) {
// Backward compatibility, the user is explicitly managing closed.
if (this._writableState) {
this._writableState.closed = value;
}
}
},

writable: {
get() {
const w = this._writableState;
Expand Down
5 changes: 0 additions & 5 deletions lib/internal/fs/streams.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,9 @@ function _construct(callback) {

function close(stream, err, cb) {
if (!stream.fd) {
// TODO(ronag)
// stream.closed = true;
cb(err);
} else {
stream[kFs].close(stream.fd, (er) => {
stream.closed = true;
cb(er || err);
});
stream.fd = null;
Expand Down Expand Up @@ -143,7 +140,6 @@ function ReadStream(path, options) {
this.end = options.end;
this.pos = undefined;
this.bytesRead = 0;
this.closed = false;
this[kIsPerformingIO] = false;

if (this.start !== undefined) {
Expand Down Expand Up @@ -327,7 +323,6 @@ function WriteStream(path, options) {
this.start = options.start;
this.pos = undefined;
this.bytesWritten = 0;
this.closed = false;
this[kIsPerformingIO] = false;

if (this.start !== undefined) {
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-fs-read-stream-inherit.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ const rangeFile = fixtures.path('x.txt');
file.on('error', common.mustCall());

process.on('exit', function() {
assert(!file.closed);
assert(file.closed);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

brings this inline with autoDestroy enabled by default

assert(file.destroyed);
});
}
2 changes: 1 addition & 1 deletion test/parallel/test-fs-read-stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ if (!common.isWindows) {
file.on('error', common.mustCall());

process.on('exit', function() {
assert(!file.closed);
assert(file.closed);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

brings this inline with autoDestroy enabled by default

assert(file.destroyed);
});
}
13 changes: 12 additions & 1 deletion test/parallel/test-stream-readable-destroy.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,21 @@ const assert = require('assert');

{
const read = new Readable({
read() {}
read() {},
destroy(err, cb) {
assert.strictEqual(read.closed, false);
cb();
assert.strictEqual(read.closed, true);
}
});
read.resume();

read.on('close', common.mustCall());

assert.strictEqual(read.closed, false);
read.destroy();
assert.strictEqual(read.destroyed, true);
assert.strictEqual(read.closed, true);
}

{
Expand Down Expand Up @@ -101,7 +108,9 @@ const assert = require('assert');
assert.strictEqual(err, null);
process.nextTick(() => {
this.push(null);
assert.strictEqual(read.closed, false);
cb();
assert.strictEqual(read.closed, true);
});
});

Expand All @@ -112,9 +121,11 @@ const assert = require('assert');

read.destroy();

assert.strictEqual(read.closed, false);
read.removeListener('end', fail);
read.on('end', common.mustNotCall());
assert.strictEqual(read.destroyed, true);
assert.strictEqual(read.closed, false);
}

{
Expand Down
12 changes: 11 additions & 1 deletion test/parallel/test-stream-writable-destroy.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,21 @@ const assert = require('assert');

{
const write = new Writable({
write(chunk, enc, cb) { cb(); }
write(chunk, enc, cb) { cb(); },
destroy(err, cb) {
assert.strictEqual(write.closed, false);
cb();
assert.strictEqual(write.closed, true);
}
});

write.on('finish', common.mustNotCall());
write.on('close', common.mustCall());

assert.strictEqual(write.closed, false);
write.destroy();
assert.strictEqual(write.destroyed, true);
assert.strictEqual(write.closed, true);
}

{
Expand Down Expand Up @@ -113,7 +120,9 @@ const assert = require('assert');
assert.strictEqual(err, null);
process.nextTick(() => {
this.end();
assert.strictEqual(write.closed, false);
cb();
assert.strictEqual(write.closed, true);
});
});

Expand All @@ -127,6 +136,7 @@ const assert = require('assert');
write.removeListener('finish', fail);
write.on('finish', common.mustCall());
assert.strictEqual(write.destroyed, true);
assert.strictEqual(write.closed, false);
}

{
Expand Down