Memory Leak disposing oldProgram

[cplepage]

🔎 Search Terms

memory leak

🕗 Version & Regression Information

• This is a crash

⏯ Playground Link

No response

💻 Code

// Your code here

🙁 Actual behavior

In safari (Webkit/JavaScriptCore). Old Program never gets GCed because of reference on it.

Memory build up really quickly.

🙂 Expected behavior

Unreferenced and gc

Additional information about the issue

No response

[cplepage]

Safari web inspector, running TS language service to getDiagnostics on file update.

without fix:

with fix:

[RyanCavanaugh]

Accepting PRs to fix memory leaks, but we obviously can't really investigate something without a repro

[cplepage]

Ryan Cavanaugh (@RyanCavanaugh) definitely, I will build a little something to make this issue reproducible and figure out what could go on. But I will need some time. Please allow me 1-2 week and I will come back here with a clear workable playground.

[tmm1]

Friendly ping on repro

[cplepage]

Yes sorry about going radio silent here.

Since discovering this issue and digging deep enough to believe the problem lays deep inside WebKit/JavaScriptCore, I simply patched it with my dumb fix https://github.com/fullstackedorg/editor/blob/main/build.ts#L9

To reproduce the issue, it needs a full in-browser setup of loading tons of files (node_modules) and requesting the language host repeatedly solely in-browser. I haven't found any browser IDE or code-editor which does that and is more than a playground which implies only a few files.

If you have any example of fully in-browser code-editor/IDE that allows to load packages from say npm of added by .tgz, I would be glad to check if the issue is reproducible in any of those.

The project I'm working on, FullStacked, relies on that. For the last couple months, I've been working heavily on reaching a production ready stability, so I haven't put much thoughts on this issue. But you could try spinning up the project and simply compare in Safari the behaviour if you comment or not the code block previously mentioned here.

There is no up-to-date contribution or how to build from source guide, so if you dive into that path, you'll probably need to figure out and modify some stuff. Although, I'm always available for any questions on the repo!

I will hopefully invest some time in documentation and said guides shortly. As soon as I can send a straightforward way to reproduce, I will do that here for sure. Also, send me links of app/IDE/code-editor that runs in-browser with the possibility of loading complete libraries with dependencies. I'll help as much as possible on the issue.

[tmm1]

Very interesting, thanks for the details.

There is another way to run tsserver with JSC, and in-fact I have observed similar memory growth issues in that environment- much more so than the equivalent code running in V8.

In VSCode:

[cplepage]

Yes!! Aman Karmani (@tmm1) You are one clever guy!

The issue seems to be the same. The oldProgram being retained only in JSC.

Here's the repo to test : https://github.com/cplepage/ts-leak.git

It simply getSemanticDiagnostics every 100ms on a basic MUI tsx file. The test runs for 60s while logging the memory usage.

To run:

bun install

bun index.js

run with fix:

bun index.js --fix

run with node

node index.js

node index.js --fix

Here are the results on my end

	no fix	with fix
bun	Memory after 1m: 1.05 GB [max: 1.05 GB, min: 130 MB]	Memory after 1m: 126 MB [max: 130 MB, min: 124 MB]
node	Memory after 1m: 383 MB [max: 528 MB, min: 161 MB]	Memory after 1m: 311 MB [max: 528 MB, min: 167 MB]

% bun --version 
1.1.45

% node --version
v20.14.0

% system_profiler SPHardwareDataType
Hardware:

    Hardware Overview:

      Model Name: MacBook Pro
      Model Identifier: MacBookPro15,4
      Processor Name: Quad-Core Intel Core i5
      Processor Speed: 1.4 GHz
      Number of Processors: 1
      Total Number of Cores: 4
      L2 Cache (per Core): 256 KB
      L3 Cache: 6 MB
      Hyper-Threading Technology: Enabled
      Memory: 16 GB
      System Firmware Version: 2069.40.2.0.0 (iBridge: 22.16.11072.0.0,0)
      OS Loader Version: 582~2132
      Activation Lock Status: Enabled

[tmm1]

Wonderful we have a repro! What a dramatic difference on the bun side.

Ryan Cavanaugh (@RyanCavanaugh) wdyt

[tmm1]

My results:

	no fix	fix
bun 1.1.45	Memory after 1m: 1.7 GB [max: 1.7 GB, min: 148 MB]	Memory after 1m: 127 MB [max: 131 MB, min: 127 MB]
node 18.17.1	Memory after 1m: 445 MB [max: 536 MB, min: 194 MB]	Memory after 1m: 304 MB [max: 534 MB, min: 190 MB]
node 20.18.1	Memory after 1m: 518 MB [max: 518 MB, min: 193 MB]	Memory after 1m: 511 MB [max: 524 MB, min: 193 MB]

[maschwenk]

That repro is very interesting. We have a similar problem where specifically on an app that also depends on

    "@emotion/react": "^11.14.0",
    "@emotion/styled": "^11.14.0",

Can consume 9-10 GB in < a minute. I don't know if it'd technically be called a leak because it does not grow indefinitely though.

I've tried applying this fix and does not seem to help, so maybe unrelated, but I'm sort of curious if the choice of mui/@emotion was on purpose? I've also noticed that even after all files are closed in VScode, memory will be pinned to that 9-10GB number indefinitely (which does seem a bit leaky?). This https://github.com/cplepage/ts-leak repro has inspired me to maybe make my own.

[jakebailey]

I've also noticed that even after all files are closed in VScode, memory will be pinned to that 9-10GB number indefinitely (which does seem a bit leaky?).

It's doing that intentionally (the keeping in memory part, not the 10GB part), since you may reopen a file. If you close all your files, then open a file, it's not so good to have that be a complete restart.

That being said, I don't know how you can possibly see 9-10GB of memory usage; we have a hard cap when run via VS Code of 4GB, but maybe you have instructed VS Code to use the "real" node instead with different max space flags, but then surely that's a different bug to this JSC problem.

This https://github.com/cplepage/ts-leak repro has inspired me to maybe make my own.

More repros are of course, good.

[jakebailey]

FWIW I still do not think #58138 itself is the right fix, just from an implementation perspective; it's possible there's a similar change that would work fine. But I really do wonder if this is something JSC should be fixing given V8 does not have this problem.

Note that in #58137 (comment), Node's (V8's) min/max memory usage is the same before and after the change (I would not focus on a specific measurement since that really depends on when the last GC run ran among other things).