Skip to content
This repository was archived by the owner on Apr 22, 2023. It is now read-only.

stream.Writable {highWaterMark: 1} crashes if written to via pipe() >= 1000 times in the same tick #5337

Closed
deoxxa opened this issue Apr 19, 2013 · 4 comments
Assignees
Milestone

Comments

@deoxxa
Copy link

deoxxa commented Apr 19, 2013

This took a little while to track down and isolate. Basically there's something "weird", or "funky" if you will, that makes things "interesting" if you somehow manage to write >1000 times (not just >1000 bytes, but >1000 times) through a stream.Readable to a stream.Writable via pipe(). This behaviour occurs on both 0.10.4 and 0.11.0. As far as I can tell, this is the absolute minimal test case for the behaviour - changing it in any significant way removes the crash.

Check it out:

var stream = require("stream");

var src = new stream.PassThrough({highWaterMark: 1});

var dst = new stream.Writable({highWaterMark: 1});
dst._write = function _write(input, encoding, done) {
  return done();
};

src.pipe(dst);

for (var i=0;i<1000;++i) {
  src.write("hello");
}

This results in:

Trace: (node) warning: Recursive process.nextTick detected. This will break in the next version of node. Please use setImmediate for recursive deferral.
    at maxTickWarn (node.js:377:17)
    at process.nextTick (node.js:480:9)
    at onwrite (_stream_writable.js:249:15)
    at WritableState.onwrite (_stream_writable.js:92:5)
    at Writable._write (/Users/conrad/work/seriousbusiness/bigquery/x.js:9:10)
    at doWrite (_stream_writable.js:211:10)
    at writeOrBuffer (_stream_writable.js:201:5)
    at Writable.write (_stream_writable.js:172:11)
    at write (_stream_readable.js:547:24)
    at flow (_stream_readable.js:556:7)
    at Writable.<anonymous> (_stream_readable.js:537:7)
    at Writable.EventEmitter.emit (events.js:92:17)
    at onwriteDrain (_stream_writable.js:270:12)
    at afterWrite (_stream_writable.js:260:5)
Trace: (node) warning: Recursive process.nextTick detected. This will break in the next version of node. Please use setImmediate for recursive deferral.
    at maxTickWarn (node.js:377:17)
    at process.nextTick (node.js:480:9)
    at onwrite (_stream_writable.js:249:15)
    at WritableState.onwrite (_stream_writable.js:92:5)
    at Socket._write (net.js:634:5)
    at doWrite (_stream_writable.js:211:10)
    at writeOrBuffer (_stream_writable.js:201:5)
    at Socket.Writable.write (_stream_writable.js:172:11)
    at Socket.write (net.js:596:40)
    at Console.warn (console.js:61:16)

<lots of repeated stuff>

Change the 1000 in the for statement to 999 though and you get no error. Putting the done() call into a setImmediate also prevents the crash. As does changing highWaterMark to anything higher than 1.

Interestingly enough, you get different behaviour if you change it to above 1000 (say, 1001). See this:

Trace: (node) warning: Recursive process.nextTick detected. This will break in the next version of node. Please use setImmediate for recursive deferral.
    at maxTickWarn (node.js:377:17)
    at process.nextTick (node.js:480:9)
    at maybeReadMore (_stream_readable.js:395:13)
    at readableAddChunk (_stream_readable.js:145:5)
    at PassThrough.Readable.push (_stream_readable.js:113:10)
    at PassThrough.Transform.push (_stream_transform.js:142:32)
    at afterTransform (_stream_transform.js:98:12)
    at TransformState.afterTransform (_stream_transform.js:76:12)
    at PassThrough._transform (_stream_passthrough.js:40:3)
    at PassThrough.Transform._read (_stream_transform.js:181:10)
    at PassThrough.Readable.read (_stream_readable.js:294:10)
    at flow (_stream_readable.js:553:52)
    at Writable.<anonymous> (_stream_readable.js:537:7)
    at Writable.EventEmitter.emit (events.js:92:17)
    at onwriteDrain (_stream_writable.js:270:12)
    at afterWrite (_stream_writable.js:260:5)
Trace: (node) warning: Recursive process.nextTick detected. This will break in the next version of node. Please use setImmediate for recursive deferral.
    at maxTickWarn (node.js:377:17)
    at process.nextTick (node.js:480:9)
    at onwrite (_stream_writable.js:249:15)
    at WritableState.onwrite (_stream_writable.js:92:5)
    at Socket._write (net.js:634:5)
    at doWrite (_stream_writable.js:211:10)
    at writeOrBuffer (_stream_writable.js:201:5)
    at Socket.Writable.write (_stream_writable.js:172:11)
    at Socket.write (net.js:596:40)
    at Console.warn (console.js:61:16)

<lots of repeated stuff>
@mks
Copy link

mks commented Apr 19, 2013

@deoxxa
Copy link
Author

deoxxa commented Apr 19, 2013

Oh, yes, I know about the process.nextTick behaviour. It's the fact that you can trigger that behaviour (causing what is effectively an infinite loop) by writing a bunch of data to a stream that I consider to be a bug.

EDIT: I didn't know that the actual number was 1000 though - I thought it was much lower. At least that solves the "why exactly 1000?!" mystery for me :)

@jasnell
Copy link
Member

jasnell commented May 26, 2015

Unable to recreate in v0.12. @deoxxa .. can you confirm that this is still an issue in v0.10.38?

@jasnell
Copy link
Member

jasnell commented May 27, 2015

Appears to have been resolved.

@jasnell jasnell closed this as completed May 27, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants