René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"Readable._read is called multiple times synchronously","articleBody":"\u003c!--\r\nThank you for reporting a possible bug in Node.js.\r\n\r\nPlease fill in as much of the template below as you can.\r\n\r\nVersion: output of `node -v`\r\nPlatform: output of `uname -a` (UNIX), or version and 32 or 64-bit (Windows)\r\nSubsystem: if known, please specify the affected core module name\r\n\r\nIf possible, please provide code that demonstrates the problem, keeping it as\r\nsimple and free of external dependencies as you can.\r\n--\u003e\r\n\r\n* **Version**: v10.10.0, v11.4.0\r\n* **Platform**: Linux 4.4.0-45-generic #66-Ubuntu SMP Wed Oct 19 14:12:37 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux\r\n* **Subsystem**: stream\r\n\r\n\u003c!-- Please provide more details below this comment. --\u003e\r\n\r\n[The documentation](https://nodejs.org/api/stream.html#stream_readable_read_size_1) for `readable._read(size)` states the following:\r\n\r\n\u003e `readable._read()` is guaranteed to be called only once within a synchronous execution, i.e. a microtick.\r\n\r\nHowever the following example demonstrates starvation if that method is able to push all of its data synchronously. The stream implementation ends up calling the read-function repeatedly and won't yield execution as documented.\r\n\r\n```JavaScript\r\nvar Readable = require('stream').Readable;\r\n\r\nlet c = 10;\r\nlet r = new Readable({\r\n    read: () =\u003e {\r\n        process.nextTick(console.log, 'next-tick');\r\n        r.push('a');\r\n    }\r\n});\r\n\r\nr.on('data', () =\u003e {});\r\nprocess.nextTick(console.log, 'Got food');  // Never printed.\r\nconsole.log('Requested food');\r\n```\r\n\r\nI believe the issue is caused by the `flow` function calling `stream.read` repeatedly within a while-loop until the stream fails to return data:\r\n\r\nhttps://github.com/nodejs/node/blob/951b1c3e09bae8395dde0d4f5cecb1fde117fa94/lib/_stream_readable.js#L933-L937\r\n\r\nOne option for fixing this might be to make `flow` recurse with next tick:\r\n\r\n```JavaScript\r\nfunction flow(stream) {\r\n  const state = stream._readableState;\r\n  debug('flow', state.flowing);\r\n  flow_(stream, state);\r\n}\r\n\r\nfunction flow_(stream, state) {\r\n  if(state.flowing \u0026\u0026 stream.read() !== null)\r\n    process.nextTick(flow_, stream, state);\r\n}\r\n```\r\n\r\nAlternatively just updating the documentation might also work. I guess most streams that would encounter the issue would just delay `r.push()` into the next tick as a workaround.\r\n\r\nEven in my case this was just something I spotted while looking into #24918. I don't personally have a use case for such a stream.","author":{"url":"https://github.com/Rantanen","@type":"Person","name":"Rantanen"},"datePublished":"2018-12-09T10:05:57.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":2},"url":"https://github.com/24919/node/issues/24919"}