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.

Sign up for GitHub

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

Jump to bottom

Response.file has a memory leak #13

Open
ImLunaHey opened this issue Sep 15, 2023 · 7 comments
Open

Response.file has a memory leak #13

ImLunaHey opened this issue Sep 15, 2023 · 7 comments

Comments

@ImLunaHey
Copy link
Contributor

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
Copy link
Contributor Author

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

@ImLunaHey
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@sstur
Copy link
Owner

sstur commented Sep 24, 2023

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
Copy link
Contributor Author

I wonder if it's this issue oven-sh/bun#1824 (comment)

@ImLunaHey
Copy link
Contributor Author

This should be retested on the newest bun canary release.

@sstur
Copy link
Owner

sstur commented Dec 13, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@sstur @ImLunaHey