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

File handle not released after options.maxFiles check #987

Open
codeboy111 opened this issue Mar 14, 2025 · 0 comments
Open

File handle not released after options.maxFiles check #987

codeboy111 opened this issue Mar 14, 2025 · 0 comments
Assignees
Labels
Area: files Things related to handling files, names, etc. bug

Comments

@codeboy111
Copy link

Support plan

  • Which support plan is this issue covered by? (Community, Sponsor, Enterprise): Community
  • Currently blocking your project/work? (yes/no): no
  • Affecting a production system? (yes/no): no

Context

  • Node.js version: 18.20.4
  • Release Line of Formidable (Legacy, Current, Next): latest
  • Formidable exact version: 3.5.2
  • Environment (node, browser, native, OS): node
  • Used with (popular names of modules):

What are you trying to achieve or the steps to reproduce?

I am setting options.maxFiles to 1 to ensure that only 1 file is received per request. When I am testing this, I find that if the request carries more files than options.maxFiles, some file handles created by formidable are not released until the nodejs process is terminated.

import express from 'express'
import formidable from 'formidable'
import fs from 'fs'

const app = express()
app.post('/', (req, res, next) => {
  const form = formidable({
    maxFiles: 1,
    uploadDir: './uploads',
    keepExtensions: true,
  })
  const filePaths = []
  form.on('fileBegin', (_, file) => {
    filePaths.push(file.filepath)
  })
  form.parse(req)
      .then(() => res.json('success'), err => res.json(err.message))
      .then(() => {
        setImmediate(() => {
          filePaths.forEach(path => {
            fs.promises.unlink(path)
                .then(() => {
                  console.log(`unlink ${path}`)
                }, console.log)
          })
        })
      })
})

app.listen(8080)

Here's what I found.

The function _handlePart does thing like this:

  1. emit the fileBegin event
  2. open file (creating the write stream)
  3. push file to this.openedFiles

How the bug comes:

  1. files received and fileBegin event emitted, causing the maxFiles check to emit an error
  2. formidable destroys files in this.openedFiles due to the error
  3. _handlePart goes on and write streams are still opened even if there is an error occurred
  4. those file handles can never be closed

I think maybe the solution is checking this.error between emitting the fileBegin event and file.open() in _handlePart

What was the result you got?

Files are deleted but the file handles are not released until the termination of the nodejs process.
On windows, those files are still shown in the file explorer and can not be opened/deleted.
On linux, the result of "lsof" is something like:
1 /usr/local/bin/node 27 /usr/local/app/uploads/67656e2415a12ae1195c86f01.jpg (deleted)

What result did you expect?

Files handles are released when formidable finishes its job, with or without errors

@codeboy111 codeboy111 added the bug label Mar 14, 2025
@GrosSacASac GrosSacASac self-assigned this Mar 17, 2025
@GrosSacASac GrosSacASac added the Area: files Things related to handling files, names, etc. label Mar 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: files Things related to handling files, names, etc. bug
Projects
None yet
Development

No branches or pull requests

2 participants