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

Streams memory leak #12198

Closed
oguimbal opened this issue Jun 27, 2024 · 3 comments · Fixed by #16349
Closed

Streams memory leak #12198

oguimbal opened this issue Jun 27, 2024 · 3 comments · Fixed by #16349
Assignees
Labels
bug Something isn't working confirmed bug We can reproduce this issue memory leak

Comments

@oguimbal
Copy link
Contributor

oguimbal commented Jun 27, 2024

What version of Bun is running?

1.1.17+bb66bba1b

What platform is your computer?

Darwin 23.1.0 arm64 arm

What steps can reproduce the bug?

Execute the below code, and watch memory usage:

setInterval(() => {
    Bun.gc(true);
}, 100);

const p = Bun.spawn(['cat'], {
    stdin: 'pipe',
    stdout: 'pipe',
})


const r = p.stdout.getReader()
while (true) {
    const buf = new Uint8Array(1_000_000);
    await p.stdin.write(buf);

    let i = 0;
    while (true) {
        const { value } = await r.read();
        i += value?.length ?? 0;
        if (i >= buf.length) {
            break;
        }
    }
}

What is the expected behavior?

Each iteration just wait until the buffer is streamed through cat, so I'd expect memory not to increase.

What do you see instead?

CPU 100%, memory increases rapidly.

Interestingly enough, when every byte has been streamed, if I add another .read() (which rightfuly returns no data), the memory leak disapears:

setInterval(() => {
    Bun.gc(true);
}, 100);

const p = Bun.spawn(['cat'], {
    stdin: 'pipe',
    stdout: 'pipe',
})


const r = p.stdout.getReader()
while (true) {
    const buf = new Uint8Array(1_000_000);
    await p.stdin.write(buf);

    let i = 0;
    while (true) {
        const { value } = await r.read();
        i += value?.length ?? 0;
        if (i >= buf.length) {
           // 👉  this fixes the leak ! (and never throws)
            const { value: value2 } = await r.read();
            if (value2?.length) {
                throw new Error('Expected EOF');
            }
            break;
        }
    }
}

Additional information

This issue is kind of linked to #12194 (which I discovered investigating this leak)

@oguimbal oguimbal added bug Something isn't working needs triage labels Jun 27, 2024
@oguimbal oguimbal changed the title FileSink => memory leak Streams memory leak Jun 27, 2024
@billywhizz billywhizz self-assigned this Jul 8, 2024
@Jarred-Sumner
Copy link
Collaborator

Confirmed I am able to reproduce this memory leak in Bun v1.1.32

@nektro nektro added memory leak confirmed bug We can reproduce this issue labels Oct 22, 2024
@DonIsaac
Copy link
Contributor

I'm pretty sure I've found it.

When called on a .temporary result, StreamResult.toJS allocates a new buffer and copies the data over. The original ByteList is then never de-allocated.

@Jarred-Sumner
Copy link
Collaborator

This appears to be a memory leak in ReadableStream with type "bytes".

RSS Version
110 MB node:stream (Bun)
402 MB node:stream (Node.js)
526 MB ReadableStream (Bun)

When using child_process in bun instead:

[  515.66ms] RSS 107 MB
[ 1015.64ms] RSS 109 MB
[ 1516.06ms] RSS 109 MB
[ 2016.07ms] RSS 109 MB
[ 2516.13ms] RSS 109 MB
[ 3016.16ms] RSS 109 MB
[ 3516.16ms] RSS 109 MB
[ 4016.22ms] RSS 109 MB
[ 4516.21ms] RSS 109 MB
[ 5016.24ms] RSS 110 MB
[ 5516.27ms] RSS 110 MB
[ 6016.40ms] RSS 110 MB
[ 6516.42ms] RSS 110 MB
[ 7016.42ms] RSS 110 MB
[ 7516.50ms] RSS 110 MB
[ 8016.59ms] RSS 110 MB
[ 8516.61ms] RSS 110 MB
[ 9016.69ms] RSS 110 MB
[ 9516.68ms] RSS 110 MB
RSS 110 MB
Writes 7685

Code:

let writes = 0;
setInterval(() => {
  console.log(
    `[${performance.now().toFixed(2).padStart(8)}ms]`,
    "RSS",
    (process.memoryUsage().rss / 1024 / 1024) | 0,
    "MB"
  );
}, 500);

setTimeout(() => {
  console.log("RSS", (process.memoryUsage().rss / 1024 / 1024) | 0, "MB");
  console.log("Writes", writes);
  process.exit(0);
}, 10000);

import { spawn } from "node:child_process";

const p = spawn("cat", [], {
  stdio: ["pipe", "pipe", "ignore"],
});

let i = 0;
let resolve, promise;
let buf;
p.stdout.on("data", (chunk) => {
  i += chunk.length;
  if (i >= buf.length) {
    resolve();
  }
});

while (true) {
  ({ promise, resolve } = Promise.withResolvers());
  buf = new Uint8Array(1024 * 1024 * 8);
  buf.fill(42);
  let { resolve: resolve2, promise: promise2 } = Promise.withResolvers();
  p.stdin.write(buf, () => resolve2());
  await Promise.all([promise, promise2]);
  writes++;
}

Compared to:

 bun /Users/jarred/Desktop/leak.js
[  507.79ms] RSS 215 MB
[ 1007.72ms] RSS 247 MB
[ 1507.70ms] RSS 327 MB
[ 2007.74ms] RSS 409 MB
[ 2507.82ms] RSS 411 MB
[ 3007.83ms] RSS 420 MB
[ 3507.92ms] RSS 428 MB
[ 4007.98ms] RSS 441 MB
[ 4507.99ms] RSS 450 MB
[ 5008.03ms] RSS 452 MB
[ 5508.01ms] RSS 461 MB
[ 6008.06ms] RSS 461 MB
[ 6508.26ms] RSS 478 MB
[ 7008.32ms] RSS 481 MB
[ 7508.49ms] RSS 481 MB
[ 8008.54ms] RSS 483 MB
[ 8508.61ms] RSS 499 MB
[ 9008.56ms] RSS 500 MB
[ 9508.59ms] RSS 526 MB
RSS 535 MB
Writes 7193

For the below code:

setInterval(() => {
  console.log(
    `[${performance.now().toFixed(2).padStart(8)}ms]`,
    "RSS",
    (process.memoryUsage().rss / 1024 / 1024) | 0,
    "MB"
  );
}, 500);
let writes = 0;
setTimeout(() => {
  console.log("RSS", (process.memoryUsage().rss / 1024 / 1024) | 0, "MB");
  console.log("Writes", writes);
  process.exit(0);
}, 10000);

const p = Bun.spawn(["cat"], {
  stdin: "pipe",
  stdout: "pipe",
  stderr: "ignore",
});

const r = p.stdout.getReader();
while (true) {
  const buf = new Uint8Array(1024 * 1024 * 8);
  buf.fill(42);
  await p.stdin.write(buf);
  await p.stdin.flush();

  let i = 0;
  while (true) {
    const { value } = await r.read();
    i += value?.length ?? 0;
    if (i >= buf.length) {
      break;
    }
  }
  writes++;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working confirmed bug We can reproduce this issue memory leak
Projects
None yet
5 participants