Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CreateReadStream doesn't close the file #1180

Closed
petersirka opened this issue Mar 17, 2015 · 20 comments
Closed

CreateReadStream doesn't close the file #1180

petersirka opened this issue Mar 17, 2015 · 20 comments
Labels
fs Issues and PRs related to the fs subsystem / file system. stream Issues and PRs related to the stream subsystem.

Comments

@petersirka
Copy link

I found a big problem in streams. I performed multiple tests on Ubuntu 14.04 LTS and OSX too and I see multiple opened files through lsof -p PID. I use autoClose: true (in default) and I destroy the stream manually with this module https://github.com/stream-utils/destroy.

This problem is not recorded for every request, but only if I performed multiple requests at the same time. My website falls down.

How can I prevent this problem? Is there a bug? How to close opened files? Extending ulimit is not a solution.

Thanks for any advice.

Screenshot

PS: io.js v1.5.1
I reported the same problem on node.js GitHub.

@mscdex
Copy link
Contributor

mscdex commented Mar 17, 2015

Can you show the relevant code, or at least a minimal, reproducible example?

@vkurchatkin
Copy link
Contributor

There is a known problem when you pipe a stream to a response and the client aborts request. Try this package: https://github.com/jshttp/on-finished

@petersirka
Copy link
Author

@mscdex
here is a video with low quality:
https://www.dropbox.com/s/3avchxknfc2nrxj/nodejs-iojs-stream-problem.mov
Example: https://www.dropbox.com/s/fl3ic54zw3zdglb/iojs-stream-problem.zip

@vkurchatkin the problem still persists, but the simulation is harder. Here is a video:
https://www.dropbox.com/s/p0soq6289yn5o6h/nodejs-iojs-problem-destroy-onfinished.mov
Example: https://www.dropbox.com/s/5wga3la8j10zq7y/iojs-stream-problem-harder.zip

@vkurchatkin I tested this problem more. I destroyed the stream via stream.close() and stream.destroy(), but the file has been still opened.

Thanks!

@mscdex mscdex added fs Issues and PRs related to the fs subsystem / file system. stream Issues and PRs related to the stream subsystem. investigate labels Mar 18, 2015
@jonathanong
Copy link
Contributor

please don't ask people to download zip files to test your code. just show us a gistfile.

the second example is incorrect.

var destroy = require('destroy');
var http = require('http');
var onFinished = require('on-finished');
var fs = require('fs');

http.createServer(function (req, res) {
    res.writeHead(200, { 'Content-Type': 'image/jpeg' });
    var stream = fs.createReadStream('bg.jpg');
    stream.pipe(res);
    onFinished(res, function(err) {
        res.on('finish', function() {
            destroy(stream);
        });
    });
}).listen(1337, '127.0.0.1');

console.log(process.argv[0] + ': ' + process.pid + '\nhttp://127.0.0.1:1337');

you're basically doing something like:

res.on('finish', =>
  res.on('finish', =>
    destroy(stream)
  )
)

which isn't going to work because finish is only going to be destroyed once. you just have to do this:

onFinished(res, function () {
  destroy(stream)
})

if the above doesn't work, please file an issue with on-finished and destroy.

@jonathanong
Copy link
Contributor

@petersirka
Copy link
Author

@jonathanong ohh sorry, my mistake.
I'll test it today and I'll give you a feedback. Thanks!

@petersirka
Copy link
Author

I have tested it and with on-finished it works correctly. But I wonder why the stream is not closed after the end of the reading (with autoClose: true)? So I detected: the stream must be manually closed after the pipe is closed.

I think that it's a bug. The test result with onFinished:

  • streams opened: 311 504
  • still opened: 7 files

Can you explain to me, why does it behaving like that?
Thanks.

@Trott
Copy link
Member

Trott commented Feb 17, 2016

This has been dormant for a long time. Does anyone know if it is a confirmed bug? If so, any idea if it's been fixed?

@jasnell
Copy link
Member

jasnell commented Apr 2, 2016

Without further confirmation it's difficult to say. I'd recommend closing and we can reopen later if necessary.

@jasnell
Copy link
Member

jasnell commented Apr 10, 2016

Going to close given the lack of any further confirmation or discussion. Can reopen if necessary.

@jasnell jasnell closed this as completed Apr 10, 2016
@therightstuff
Copy link

i believe this is the correct issue to post under...

i have a read stream piping a binary file to the response object,

var rstream = fs.createReadStream(testFilePath, { encoding: 'base64' });

on first call it sends the complete file to the client but doesn't close the stream, on the second call it fails but raises the close event. once the close event has been fired, the following call will succeed and the subsequent call will fail in the same way.
at no time are the 'error', 'end' or 'finish' events raised,

rstream
                .on('error', function (e) { console.log('rstream error'); })
                .on('end', function (e) { console.log('rstream end'); })
                .on('finish', function (e) { console.log('rstream finish'); })
                .on('close', function (e) { console.log('rstream close'); })
            ;


rstream
                .pipe(res)
                .on('error', function (e) { console.log('pipe error'); })
                .on('end', function (e) { console.log('pipe end'); })
                .on('finish', function (e) { console.log('pipe finish'); })
                .on('close', function (e) { console.log('pipe close'); })
            ;

in the logs, the only line is "pipe close" from the second call.

this only happens if encoding is set to base64, when i transport the file without transformation it works perfectly.

@therightstuff
Copy link

alright, i've figured out why it was only happening every second call - that's because the client's ajax calls weren't disconnecting properly (jquery). the only thing that still concerns me is that the end and finish events aren't raised, otherwise the close event is being raised just fine.

@mcollina
Copy link
Member

mcollina commented Jan 5, 2017

You should really use http://npm.im/pump to pipe things, or those situations can happen.

@therightstuff
Copy link

alright, i used pump, and it's easier to use (thanks!) but i have the same result. the only difference is that i'm able to catch the error properly on the second call (or any other call made using the jquery ajax function):

send complete Error: premature close
at ServerResponse.onclose (C:\myproject\node_modules\end-of-stream\index.js:44:54)
at emitNone (events.js:91:20)
at ServerResponse.emit (events.js:185:7)
at Socket.onServerResponseClose (_http_server.js:131:44)
at emitOne (events.js:101:20)
at Socket.emit (events.js:188:7)
at Pipe._handle.close [as _onclose] (net.js:498:12)

@mcollina
Copy link
Member

mcollina commented Jan 9, 2017

Can you please upload a full example to demonstrate your issue?

@therightstuff
Copy link

therightstuff commented Jan 10, 2017

            var rstream = fs.createReadStream(testFilePath, { encoding: 'base64' });

            pump(rstream, res, function (err) {
                console.log('send complete', err);
            });

the first call returns the file data, but doesn't close the pipe. the second call fails and produces the following error:

send complete Error: premature close
    at ServerResponse.onclose (C:\proj\node_modules\end-of-stream\index.js:44:54)
    at emitNone (events.js:91:20)
    at ServerResponse.emit (events.js:185:7)
    at Socket.onServerResponseClose (_http_server.js:131:44)
    at emitOne (events.js:101:20)
    at Socket.emit (events.js:188:7)
    at Pipe._handle.close [as _onclose] (net.js:498:12)

this is due to the jquery ajax call not completing properly, but clearly the stream should be closing regardless? again, no "end" of "finish" events raised.

@mcollina
Copy link
Member

You mention an Ajax call, can you upload a full example to demonstrate your problem?

@therightstuff
Copy link

$.ajax({
        cache: false,
        url: fileAccessUrl,
        method: 'GET',
        headers: {},
        success: function (data, textStatus, jqXHR) {
                var contentType = jqXHR.getResponseHeader("content-type");
                console.log('received data with content type ' + contentType + ' and length ' + data.length);
        },
        error: function (jqXHR, textStatus, errorThrown) {
            console.log(jqXHR.status + ':');
            console.log(textStatus);
            console.log(errorThrown);
        }
    });

@mcollina
Copy link
Member

@therightstuff I need something I can run on my laptop to help you. That's what I mean by a "Full" example.

@therightstuff
Copy link

without sharing proprietary code i would have to write it new, thanks anyway.

warren-bank added a commit to warren-bank/node-serve that referenced this issue Dec 23, 2021
purpose:
* ensure that file descriptors are closed, and never leak
  - if a fd were to leak as the result of the file
    having been requested by a client and served by httpd,
    then the underlying file will remain open
    and cause the OS to prevent the file from being deleted
    until the process running `serve` terminates

potential causes for fd leak:
 1) bug in `serve`
      https://github.com/vercel/serve-handler/blob/6.1.3/src/index.js#L751
    summary:
    - the fd is opened BEFORE checking ETag
      to conditionally return a 304 Not Modified Response
    - when this occurs,
      the fd is never piped to the response,
      and it is never closed
 2) bug in `nodejs`
      nodejs/node#1180
      nodejs/node#1834
    summary:
    - if the client closes the connection
      before the piped fd stream is fully written to the response,
      it has been reported that the fd is never closed
    workaround:
      https://github.com/jshttp/on-finished#example
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system. stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

No branches or pull requests

9 participants