Skip to content

Conversation

@Khez
Copy link

@Khez Khez commented Jan 14, 2021

We have seen a large number of open FD's when using express with multer.

After investing, we have found the issue to be aborted uploads.

It seems the upload FD is not being closed if the client aborts the request mid-way.

How To Reproduce

From the readme (adapted to a single field):

const express = require('express');
const multer  = require('multer');
const upload = multer({ dest: 'uploads/' });

var app = express();
var cpUpload = upload.fields([{
    name: 'avatar',
    maxCount: 1
}]);

app.post('/upload', cpUpload, function (req, res, next) {
    res.send('ok');
});

app.listen(8081, () => {
    console.log('Listening');
});

Using whichever client, upload a file and abort it during transit:

# make sure the file is somewhat large
# and don't forget to abort before getting the response
curl -vvv 'http://localhost:8081/upload' -F 'avatar=@file' --limit-rate 1M;

Finally using lsof on the node process, you should see a FD leaked to the uploads directory

Related(?)

Khez added a commit to Khez/multer that referenced this pull request Jan 14, 2021
@Khez Khez changed the title Close write stream on connection error Close write stream on connection error (FD leak on incomplete uploads) Jan 14, 2021
@Khez
Copy link
Author

Khez commented Mar 16, 2021

I'm saddened to see this ignored. This was a very visible issue and with a clear solution.

We attempted a similar fix in production, but we still had some FD Leaks, uncertain why. It looked something like:

const express = require('express');
const multer  = require('multer');
const upload = multer({ dest: 'uploads/' });

var app = express();
var cpUpload = upload.fields([{
    name: 'avatar',
    maxCount: 1
}]);


app.post('/upload', function (req, res, next) {
    // Userland fix attempt for https://github.com/expressjs/multer/pull/971
    (req.connection || req.socket).on('error', (error) => {
        console.log('Connection Error Detected', error.message || error);
        req.emit('end');
    });

    cpUpload(req, res, function(err) {
        console.log(err || 'Upload Done');
        res.send('ok');
    });
});

app.listen(8081, () => {
    console.log('Listening');
});

Due to this PR gathering dust and the fact that we still had FD leaks, we ended up moving to formidable. Haven't had any leaks since then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants