File handling seems to leak memory.

[Ray Morgan]

Given a simple script such as:

node.http.createServer(function (request, response) {
node.fs.cat("foo.txt", "binary")
.addCallback(function (contents) {
response.sendHeader(200, {
"Content-Type": "text/plain",
"Content-Length": contents.length
});
response.sendBody(contents);
response.finish();
});
}).listen(8000);

After booting it, it was using about 5.3MB of ram.

Then I ran:
ab -c 20 -n 10000 http://127.0.0.1:8000/

Node was then using a stable (not being GC'ed) 129.5MB of ram.

I have also tried using the new File object, to see if it was just a
bug in cat, but it had the same issue, so I am assuming this is a bug
lower down.

I am on Snow Leopard, and just did a fresh build booted into the 32bit
kernel mode.

[Ryan Dahl]

Yes it does appear to be a problem. I'll get back to you

Off topic: if serving a static file (especially if you're implementing
a static file server for a web framework) it would be best not to
buffer the entire file in memory as node.fs.cat() does but rather
stream it from the disk to the client using node.fs.read() with
multiple res.sendBody() calls. node.fs.stat() can be used to get the
file size, or you can leave out the "Content-Length" header and node
will used chunked encoding.

[Brian Hammond]

Or better yet, use nginx to serve static files.

On Sep 22, 8:59 am, Ryan Dahl <coldredle...@gmail.com> wrote:

> On Mon, Sep 21, 2009 at 9:19 PM, Ray Morgan <raycmor...@gmail.com> wrote:
>
> > Given a simple script such as:
>
> > node.http.createServer(function (request, response) {
> >  node.fs.cat("foo.txt", "binary")
> >    .addCallback(function (contents) {
> >      response.sendHeader(200, {
> >        "Content-Type": "text/plain",
> >        "Content-Length": contents.length
> >      });
> >      response.sendBody(contents);
> >      response.finish();
> >    });
> > }).listen(8000);
>
> > After booting it, it was using about 5.3MB of ram.
>
> > Then I ran:

> > ab -c 20 -n 10000http://127.0.0.1:8000/

[Ray Morgan]

Thanks for the tips. The actual implementation isn't buffering, just
wanted to show a simple example to show what looks like a memory leak.

I also agree with serving static files via Nginx, this is more for
easing local development.

On Sep 22, 5:59 am, Ryan Dahl <coldredle...@gmail.com> wrote:

> On Mon, Sep 21, 2009 at 9:19 PM, Ray Morgan <raycmor...@gmail.com> wrote:
>
> > Given a simple script such as:
>
> > node.http.createServer(function (request, response) {
> >  node.fs.cat("foo.txt", "binary")
> >    .addCallback(function (contents) {
> >      response.sendHeader(200, {
> >        "Content-Type": "text/plain",
> >        "Content-Length": contents.length
> >      });
> >      response.sendBody(contents);
> >      response.finish();
> >    });
> > }).listen(8000);
>
> > After booting it, it was using about 5.3MB of ram.
>
> > Then I ran:

> > ab -c 20 -n 10000http://127.0.0.1:8000/

[Ryan Dahl]

On Mon, Sep 21, 2009 at 9:19 PM, Ray Morgan <raycm...@gmail.com> wrote:
>
> Given a simple script such as:
>
> node.http.createServer(function (request, response) {
>  node.fs.cat("foo.txt", "binary")
>    .addCallback(function (contents) {
>      response.sendHeader(200, {
>        "Content-Type": "text/plain",
>        "Content-Length": contents.length
>      });
>      response.sendBody(contents);
>      response.finish();
>    });
> }).listen(8000);
>
> After booting it, it was using about 5.3MB of ram.
>
> Then I ran:
> ab -c 20 -n 10000 http://127.0.0.1:8000/
>
> Node was then using a stable (not being GC'ed) 129.5MB of ram.

Fixed in 76f4962.

[Ray Morgan]

Awesome! I can verify that this fixed the memory leak. Thanks.

On Sep 22, 3:20 pm, Ryan Dahl <coldredle...@gmail.com> wrote:

> On Mon, Sep 21, 2009 at 9:19 PM, Ray Morgan <raycmor...@gmail.com> wrote:
>
> > Given a simple script such as:
>
> > node.http.createServer(function (request, response) {
> >  node.fs.cat("foo.txt", "binary")
> >    .addCallback(function (contents) {
> >      response.sendHeader(200, {
> >        "Content-Type": "text/plain",
> >        "Content-Length": contents.length
> >      });
> >      response.sendBody(contents);
> >      response.finish();
> >    });
> > }).listen(8000);
>
> > After booting it, it was using about 5.3MB of ram.
>
> > Then I ran:

> > ab -c 20 -n 10000http://127.0.0.1:8000/