Skip to content

Commit 44fe78b

Browse files
kodemillMylesBorins
authored andcommitted
stream: inline needMoreData function
Inline the needMoreData function since it has only one call place. Update the related comment. Add a test for the edge case where HWM=0 and state.length=0. Add a test for ReadableStream.read(n) method's edge case where n, HWM and state.length are all zero. This proves that there is no easy way to simplify the check at https://github.com/nodejs/node/blob/master/lib/_stream_readable.js#L440Fixes: #19893 Refs: #19896 PR-URL: #21009 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Lance Ball <lball@redhat.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
1 parent 9ada68b commit 44fe78b

2 files changed

Lines changed: 55 additions & 37 deletions

File tree

‎lib/_stream_readable.js‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,11 @@ function readableAddChunk(stream, chunk, encoding, addToFront, skipChunkCheck) {
270270
}
271271
}
272272

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

276280
functionaddChunk(stream,state,chunk,addToFront){
@@ -304,19 +308,6 @@ function chunkInvalid(state, chunk) {
304308
}
305309

306310

307-
// We can push more data if we are below the highWaterMark.
308-
// Also, if we have no data yet, we can stand some
309-
// more bytes. This is to work around cases where hwm=0,
310-
// such as the repl. Also, if the push() triggered a
311-
// readable event, and the user called read(largeNumber) such that
312-
// needReadable was set, then we ought to push more, so that another
313-
// 'readable' event will be triggered.
314-
functionneedMoreData(state){
315-
return!state.ended&&
316-
(state.length<state.highWaterMark||
317-
state.length===0);
318-
}
319-
320311
Readable.prototype.isPaused=function(){
321312
returnthis._readableState.flowing===false;
322313
};
Lines changed: 50 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,58 @@
11
'use strict';
22
constcommon=require('../common');
33

4-
// This test ensures that the stream implementation correctly handles values
5-
// for highWaterMark which exceed the range of signed 32 bit integers and
6-
// rejects invalid values.
7-
84
constassert=require('assert');
95
conststream=require('stream');
106

11-
// This number exceeds the range of 32 bit integer arithmetic but should still
12-
// be handled correctly.
13-
constovfl=Number.MAX_SAFE_INTEGER;
14-
15-
constreadable=stream.Readable({highWaterMark: ovfl});
16-
assert.strictEqual(readable._readableState.highWaterMark,ovfl);
17-
18-
constwritable=stream.Writable({highWaterMark: ovfl});
19-
assert.strictEqual(writable._writableState.highWaterMark,ovfl);
20-
21-
for(constinvalidHwmof[true,false,'5',{},-5,NaN]){
22-
for(consttypeof[stream.Readable,stream.Writable]){
23-
common.expectsError(()=>{
24-
type({highWaterMark: invalidHwm});
25-
},{
26-
type: TypeError,
27-
code: 'ERR_INVALID_OPT_VALUE',
28-
message: `The value "${invalidHwm}" is invalid for option "highWaterMark"`
29-
});
7+
{
8+
// This test ensures that the stream implementation correctly handles values
9+
// for highWaterMark which exceed the range of signed 32 bit integers and
10+
// rejects invalid values.
11+
12+
// This number exceeds the range of 32 bit integer arithmetic but should still
13+
// be handled correctly.
14+
constovfl=Number.MAX_SAFE_INTEGER;
15+
16+
constreadable=stream.Readable({highWaterMark: ovfl});
17+
assert.strictEqual(readable._readableState.highWaterMark,ovfl);
18+
19+
constwritable=stream.Writable({highWaterMark: ovfl});
20+
assert.strictEqual(writable._writableState.highWaterMark,ovfl);
21+
22+
for(constinvalidHwmof[true,false,'5',{},-5,NaN]){
23+
for(consttypeof[stream.Readable,stream.Writable]){
24+
common.expectsError(()=>{
25+
type({highWaterMark: invalidHwm});
26+
},{
27+
type: TypeError,
28+
code: 'ERR_INVALID_OPT_VALUE',
29+
message:
30+
`The value "${invalidHwm}" is invalid for option "highWaterMark"`
31+
});
32+
}
33+
}
34+
}
35+
36+
{
37+
// This test ensures that the push method's implementation
38+
// correctly handles the edge case where the highWaterMark and
39+
// the state.length are both zero
40+
41+
constreadable=stream.Readable({highWaterMark: 0});
42+
43+
for(leti=0;i<3;i++){
44+
constneedMoreData=readable.push();
45+
assert.strictEqual(needMoreData,true);
3046
}
3147
}
48+
49+
{
50+
// This test ensures that the read(n) method's implementation
51+
// correctly handles the edge case where the highWaterMark, state.length
52+
// and n are all zero
53+
54+
constreadable=stream.Readable({highWaterMark: 0});
55+
56+
readable._read=common.mustCall();
57+
readable.read(0);
58+
}

0 commit comments

Comments
 (0)