fastify / fastify-multipart

Multipart support for Fastify
MIT License
479 stars 103 forks source link

toBuffer method causing memory leak #543

Closed Pooyahmti closed 1 month ago

Pooyahmti commented 1 month ago

Prerequisites

Fastify version

4.28.0

Plugin version

8.3.0

Node.js version

18.20.4

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

Ubuntu 20.04

Description

Accumulating files in memory using .toBuffer() leads to increased memory usage (RSS) due to the persistence of created buffers when handling multiple file streams.

MRE

import fastify from 'fastify';
import multipart from '@fastify/multipart';

const fastify = fastify();
fastify.register(multipart);

fastify.post('/', async (req, rep) => {
  const parts = await req.parts();

  for await (const part of parts) {
    console.log(part.type, part.fieldname)
    if (part.type === 'file') {
      console.log(part.filename,await part.toBuffer())
    }
  }
  return { hello: 'world' }
})

const start = async () => {
  try {
    await fastify.listen({ port: 3000 });
    console.log('Server listening on 3000 ...')
  } catch (err) {
    fastify.log.error(err)
    process.exit(1)
  }
}
start()

Link to code that reproduces the bug

No response

Expected Behavior

After a route handler completes and returns a response to the client, any unused buffers should be automatically garbage collected since they are no more referenced.

gurgunday commented 1 month ago

Curious: did you have a server crash or is it just increased memory usage?

Pooyahmti commented 1 month ago

While testing my app grew from 57mb to 800mb memory usage ( Monitoring on pm2), I didn't continue further. Also I've tried setting --max-old-space-size flag for node. but no effect ( I guess that is only for heap).

mcollina commented 1 month ago

so... toBuffer() accumulates things in memory. That's the whole point of it... don't use if it's a problem for you.

Pooyahmti commented 1 month ago

Sorry, this is busboy's behaviour, should have opened the issue there. @mcollina Pretty expert advice btw ;)