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

FDs should only open once a file stream is being consumed #6782

Closed
jonathanong opened this issue Dec 30, 2013 · 6 comments
Closed

FDs should only open once a file stream is being consumed #6782

jonathanong opened this issue Dec 30, 2013 · 6 comments

Comments

@jonathanong
Copy link

example: https://gist.github.com/jonathanong/8186604

this will cause an EMFILE error, even though none of the streams are ever consumed.

@tjfontaine
Copy link

This is the correct behavior of the ReadStream by default it opens and starts reading until highWaterMark you should be handling .on('error') by default and be prepared to handle cases like EMFILE or ENOENT etc. If you want something a bit more magical about EMFILE you can look at https://npmjs.org/package/graceful-fs

In reality you should only be creating ReadStreams when you're ready to consume them, as an unsupported and kludgy workaround for you could:

var s = fs.createReadStream('foo.txt', { fd: -1 });
s.fd = null;

The current implementation details mean that since you're giving it an fd it won't try and open by default, and then the checks in _read later will attempt to see if fd is indeed valid and if not try and open upon first read, but if you're still over your limit you'll still need to handle EMFILE in the normal way.

@jonathanong
Copy link
Author

thanks for your quick response! it's just a little harder to pass around file streams and then decide not to use them, specifically when you're making a framework! not a big deal, especially with that work around!

@tj
Copy link

tj commented Dec 31, 2013

is there a reason that ReadStreams are so eager? or is that mostly to retain streams 1 support? definitely awkward api-wise

@jonathanong
Copy link
Author

the more i think about this, the more i think this is incorrect behavior. there should be a clear distinction between initialization and processing. right now, processing begins at initialization.

the API should be:

var stream = fs.createReadStream('filename.txt')
stream.read(0)

if you want events to emitted ASAP. otherwise, no processing should occur.

this won't affect the general use case where the streams are flowing immediately via .pipe or .on('data'.

would you take a PR? /cc @isaacs

@tj
Copy link

tj commented Dec 31, 2013

this would probably classify as a breaking change regardless unfortunately :( even though it's supposed to be somewhat transparent

@jeromew
Copy link

jeromew commented Jan 20, 2014

I agree with @jonathanong that beeing able to separate initialization & processing would be a win in the stream API.

I used to do a lot of SIP related things (session initiation protocol) for voice over IP and the most solid APIs were the one were you could take your time to setup / configure the session before starting it.

it is not always necessary but once you start handling complex stream graphs (you can think of gstreamer like stream graph for example where each node in the graph can adapt itself depending on the capabilities of the other connected streams), it is better to first build the stream wiring (pipe) and only after this start pumping data in the graph.

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