`Response.file` has a memory leak

ImLunaHey commented 1 year ago

Save this as src/poc.ts.

import { fileURLToPath } from 'url';
import { join } from 'path';
import { createApplication, Response } from '@nbit/bun';

const __dirname = fileURLToPath(new URL('.', import.meta.url));

const { defineRoutes, attachRoutes, createRequestHandler } = createApplication({
    root: join(__dirname, '..'),
    allowStaticFrom: ['assets'],
});

const memoryLeak = () => Response.file('assets/poc.html');

const noMemoryLeak = () => `<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="utf-8" />
    <meta http-equiv="x-ua-compatible" content="ie=edge" />
    <meta name="viewport" content="width=device-width, initial-scale=1" />
    <title>Vote</title>
  </head>
  <body>
    <span>Hello world</span>
  </body>
</html>`;

const routes = defineRoutes((app) => [app.get('/memory-leak', memoryLeak), app.get('/no-memory-leak', noMemoryLeak)]);

const port = 3000;

Bun.serve({
    port,
    fetch: attachRoutes(routes),
});

console.log(`Server running at http://localhost:${port}`);

const formatMemoryUsage = (usage: number) => `${Math.round(usage / 1024 / 1024 * 100) / 100} MB`;

setInterval(() => {
    const memoryData = process.memoryUsage();

    const memoryUsage = {
        rss: formatMemoryUsage(memoryData.rss),
        heapTotal: formatMemoryUsage(memoryData.heapTotal),
        heapUsed: formatMemoryUsage(memoryData.heapUsed),
        external: formatMemoryUsage(memoryData.external),
    };

    console.log(memoryUsage);
}, 1_000);

Save this as assets/poc.html

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="utf-8" />
    <meta http-equiv="x-ua-compatible" content="ie=edge" />
    <meta name="viewport" content="width=device-width, initial-scale=1" />
    <title>Vote</title>
  </head>
  <body>
    <span>Hello world</span>
  </body>
</html>

Start the server in one terminal with bun run ./src/poc.ts and then in a second terminal start the benchmark with bunx autocannon http://localhost:3000/no-memory-leak -c 1000

Notice a small memory increase but then it stabilises. You can run this as many time as you want after this and the memory will barely move a few MB if that.

If you now run bunx autocannon http://localhost:3000/memory-leak -c 1000 you'll see the memory creeps up into the 100s of MB and stays there. Every time you re-run it you'll keep seeing more and more memory usage.

ImLunaHey commented 1 year ago

Screen Shot 2023-09-15 at 12 28 19 pm

ImLunaHey commented 1 year ago

Yep. If I change the file to use this instead I get no leaks.

const pocFile = readFileSync('assets/poc.html', 'utf-8');
const memoryLeak = () => pocFile;

and even if I use the Response class still no leak.

const pocFile = readFileSync('assets/poc.html', 'utf-8');
const memoryLeak = () => new Response(pocFile, {
    headers: {
        'Content-Type': 'text/html; charset=utf-8',
    }
});

ImLunaHey commented 1 year ago

@sstur would you mind looking at this as it currently makes the framework somewhat unusable. 😢

sstur commented 1 year ago

I wasn't able to get to this yet, but thanks for your patience, will dig in as soon as I can carve out the time.

ImLunaHey commented 1 year ago

I wonder if it's this issue https://github.com/oven-sh/bun/issues/1824#issuecomment-1732684214

ImLunaHey commented 1 year ago

This should be retested on the newest bun canary release.

sstur commented 10 months ago

Do you know if this was fixed in bun core or if it's still an issue?

sstur / nbit

`Response.file` has a memory leak #13