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

Memory leak in Deno (on Deno versions >= 1.31.0) #18369

Closed
quentinadam opened this issue Mar 22, 2023 · 17 comments · Fixed by #18844
Closed

Memory leak in Deno (on Deno versions >= 1.31.0) #18369

quentinadam opened this issue Mar 22, 2023 · 17 comments · Fixed by #18844
Labels
bug Something isn't working upstream Changes in upstream are required to solve these issues

Comments

@quentinadam
Copy link

quentinadam commented Mar 22, 2023

The below very simple program will ostensibly (at a rate of ~1MB/second) leak memory in Deno (only on Deno versions >= 1.31.0). To reproduce, please add a hello.txt file with a random content such as Hello! at the root and run the below program:

const file = new URL('./hello.txt', import.meta.url).pathname;
while (true) {
  Deno.readFileSync(file);
}

The memory leak only happens on Deno versions >= 1.31.0. It does not happen on Deno versions <= 1.30.3.
The memory leak is more significant with shorter files.

Here is a slightly longer version of the program that will output the memory usage every second :

console.log('Deno version', Deno.version.deno);

const file = new URL('./hello.txt', import.meta.url).pathname;

let timestamp = new Date();

while (true) {
  if (Date.now() >= timestamp.valueOf()) {
    const bytes = Deno.memoryUsage().rss;
    console.log(timestamp.toISOString(), Math.floor(bytes / (1024 * 1024) * 10) / 10);
    timestamp = new Date(timestamp.valueOf() + 1000);
  }
  Deno.readFileSync(file);
}

I have also created a repository that showcases the issue: https://github.com/quentinadam/deno-memory-leak.

@quentinadam quentinadam changed the title Async loop leaks memory in Deno version >= 1.31.0 Memory leak in Deno version >= 1.31.0 Mar 22, 2023
@quentinadam quentinadam changed the title Memory leak in Deno version >= 1.31.0 Memory leak in Deno Mar 22, 2023
@quentinadam quentinadam changed the title Memory leak in Deno Memory leak in Deno (on versions >= 1.31.0) Mar 22, 2023
@quentinadam quentinadam changed the title Memory leak in Deno (on versions >= 1.31.0) Memory leak in Deno (on Deno versions >= 1.31.0) Mar 22, 2023
@dsherret dsherret changed the title Memory leak in Deno (on Deno versions >= 1.31.0) Memory leak in Deno (on Deno versions >= 1.31.0) with small files Mar 23, 2023
@dsherret dsherret added the bug Something isn't working label Mar 23, 2023
@dsherret
Copy link
Member

dsherret commented Mar 23, 2023

I wasn't initially able to reproduce this, but then I used the file you had with just "Hello!" in it and I was able to. This is not reproducible on Windows it seems.

@quentinadam
Copy link
Author

Yes, the memory leak is more obvious with small files (I should have mentioned it). Although, I think the memory leak is also present with larger files too. I have the impression the memory leak is due to the loop, hence why the memory leak is more significant when the loop is faster (when reading smaller files).

@dsherret dsherret changed the title Memory leak in Deno (on Deno versions >= 1.31.0) with small files Memory leak in Deno (on Deno versions >= 1.31.0) Mar 23, 2023
@quentinadam
Copy link
Author

The memory leak also happens with fetch. To reproduce, please first run a simple HTTP server (like the one provided on https://deno.land/manual@v1.31.1/examples/http_server) and run the following code:

console.log('Deno version', Deno.version.deno);

let timestamp = new Date();

while (true) {
  if (Date.now() >= timestamp.valueOf()) {
    const bytes = Deno.memoryUsage().rss;
    console.log(timestamp.toISOString(), Math.floor(bytes / (1024 * 1024) * 10) / 10);
    timestamp = new Date(timestamp.valueOf() + 1000);
  }
  const response = await fetch('http://localhost:8080');
  await response.text();
}

Here is a chart of the memory usage over the first 10 minutes when I run the program on MacOS ARM64 (I experience similar results on Linux x86-64):

Memory usage chart

@ry
Copy link
Member

ry commented Mar 23, 2023

V8 is quite lazy when it comes to GC. Try --v8-flags=--expose-gc and call gc() in the loop.

There could very well be a memory leak, but this plot doesn't say much.

@dsherret
Copy link
Member

@ry it still runs out of memory, but at a slower pace. I didn't test fetch though. For the fetch test, it should be run until it runs out of memory because something might be caching or utilizing system memory.

@quentinadam
Copy link
Author

@dsherret, when I run the program (the one reading the file) with --v8-flags=--expose-gc and by calling gc()in the loop, it does seem to somewhat solve the memory leak issue. Here is a chart of the memory usage over time:

Screenshot 2023-03-23 at 09 40 20

However, having to resort to calling gc() does not feel right and it is still odd that:

  • None of this behaviour happens before v1.31.0
  • Without explicitly calling gc() explicitly, the memory usage of the program will grow continuously until memory exhaustion, at which point the program will be killed.

@naegelin
Copy link

I can confirm we are seeing this also on long running Deno processes in Deno v1.32.1 .

After some time Deno exits with Cannot grow ExternalPointerTable The problem does not occur prior to 1.31.0 . We are using both fetch and readFileSync.

@lucacasonato lucacasonato self-assigned this Mar 27, 2023
@aapoalas
Copy link
Collaborator

Do you get any sort of backtrace from the crash?

@lucacasonato
Copy link
Member

Bisected, and unfortunately it's this:

65500f36e870b4ada3996b06aa287e30177d21a3 is the first bad commit
commit 65500f36e870b4ada3996b06aa287e30177d21a3
Author: Bartek Iwańczuk <biwanczuk@gmail.com>
Date:   Tue Feb 7 13:36:41 2023 +0100

    chore: upgrade rusty_v8 to 0.62.2 (#17604)
    
    Co-authored-by: Bert Belder <bertbelder@gmail.com>

 Cargo.lock                                         |   4 ++--
 Cargo.toml                                         |   2 +-
 .../run/worker_close_in_wasm_reactions.js.out      |   1 +
 core/icudtl.dat                                    | Bin 10454784 -> 10541264 bytes
 core/inspector.rs                                  |  18 ++++++++++++++++++
 core/ops_builtin_v8.rs                             |   8 ++++----
 core/runtime.rs                                    |   4 ++--
 7 files changed, 28 insertions(+), 9 deletions(-)

@bartlomieju
Copy link
Member

We'll look into the V8 upgrade problem this week.

@lucacasonato lucacasonato removed their assignment Mar 27, 2023
@bartlomieju
Copy link
Member

We're floating a V8 patch to verify if this is a V8 regression.

@lucacasonato
Copy link
Member

lucacasonato commented Mar 28, 2023

@naegelin Do you have a backtrace / crashdump for the error you are seeing? Is the code that is causing Deno to crash public? If not, are you able to share it with us privately (if so, you can email me at luca@deno.com).

@naegelin
Copy link

Hey @lucacasonato unfortunately I cannot share the code. I have restarted a process with 1.32.1 and will try to capture the crashdump. It can take several hours so I'll report back when I have something.

@jmordica
Copy link

jmordica commented Apr 7, 2023

👀

@sergeysolovev
Copy link

@bartlomieju

Thank you for the fix! This is not yet included in v1.32.5, correct?

@bartlomieju
Copy link
Member

No, it's not. I will be released later this week 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working upstream Changes in upstream are required to solve these issues
Projects
None yet
10 participants