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: inline needMoreData function, add edge-case tests #21009

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
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
19 changes: 5 additions & 14 deletions lib/_stream_readable.js
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,11 @@ function readableAddChunk(stream, chunk, encoding, addToFront, skipChunkCheck) {
}
}

return needMoreData(state);
// We can push more data if we are below the highWaterMark.
// Also, if we have no data yet, we can stand some more bytes.
// This is to work around cases where hwm=0, such as the repl.
return !state.ended &&
(state.length < state.highWaterMark || state.length === 0);
}

function addChunk(stream, state, chunk, addToFront) {
Expand Down Expand Up @@ -304,19 +308,6 @@ function chunkInvalid(state, chunk) {
}


// We can push more data if we are below the highWaterMark.
// Also, if we have no data yet, we can stand some
// more bytes. This is to work around cases where hwm=0,
// such as the repl. Also, if the push() triggered a
// readable event, and the user called read(largeNumber) such that
// needReadable was set, then we ought to push more, so that another
// 'readable' event will be triggered.
function needMoreData(state) {
return !state.ended &&
(state.length < state.highWaterMark ||
state.length === 0);
}

Readable.prototype.isPaused = function() {
return this._readableState.flowing === false;
};
Expand Down
73 changes: 50 additions & 23 deletions test/parallel/test-streams-highwatermark.js
Original file line number Diff line number Diff line change
@@ -1,31 +1,58 @@
'use strict';
const common = require('../common');

// This test ensures that the stream implementation correctly handles values
// for highWaterMark which exceed the range of signed 32 bit integers and
// rejects invalid values.

const assert = require('assert');
const stream = require('stream');

// This number exceeds the range of 32 bit integer arithmetic but should still
// be handled correctly.
const ovfl = Number.MAX_SAFE_INTEGER;

const readable = stream.Readable({ highWaterMark: ovfl });
assert.strictEqual(readable._readableState.highWaterMark, ovfl);

const writable = stream.Writable({ highWaterMark: ovfl });
assert.strictEqual(writable._writableState.highWaterMark, ovfl);

for (const invalidHwm of [true, false, '5', {}, -5, NaN]) {
for (const type of [stream.Readable, stream.Writable]) {
common.expectsError(() => {
type({ highWaterMark: invalidHwm });
}, {
type: TypeError,
code: 'ERR_INVALID_OPT_VALUE',
message: `The value "${invalidHwm}" is invalid for option "highWaterMark"`
});
{
// This test ensures that the stream implementation correctly handles values
// for highWaterMark which exceed the range of signed 32 bit integers and
// rejects invalid values.

// This number exceeds the range of 32 bit integer arithmetic but should still
// be handled correctly.
const ovfl = Number.MAX_SAFE_INTEGER;

const readable = stream.Readable({ highWaterMark: ovfl });
assert.strictEqual(readable._readableState.highWaterMark, ovfl);

const writable = stream.Writable({ highWaterMark: ovfl });
assert.strictEqual(writable._writableState.highWaterMark, ovfl);

for (const invalidHwm of [true, false, '5', {}, -5, NaN]) {
for (const type of [stream.Readable, stream.Writable]) {
common.expectsError(() => {
type({ highWaterMark: invalidHwm });
}, {
type: TypeError,
code: 'ERR_INVALID_OPT_VALUE',
message:
`The value "${invalidHwm}" is invalid for option "highWaterMark"`
});
}
}
}

{
// This test ensures that the push method's implementation
// correctly handles the edge case where the highWaterMark and
// the state.length are both zero

const readable = stream.Readable({ highWaterMark: 0 });

for (let i = 0; i < 3; i++) {
const needMoreData = readable.push();
assert.strictEqual(needMoreData, true);
}
}

{
// This test ensures that the read(n) method's implementation
// correctly handles the edge case where the highWaterMark, state.length
// and n are all zero

const readable = stream.Readable({ highWaterMark: 0 });

readable._read = common.mustCall();
readable.read(0);
}