Fix main process OOM from unconsumed buffered pty host service events - #323980

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

Robo (@deepak1556)

Matched files:

• src/vs/code/electron-main/app.ts

@microsoft-github-policy-service agree

Previous take #285419

cc Anthony Kim (@anthonykim1)

From my understanding of the context, the recommendation from Alexandru Dima (@alexdima) and others was to remove the buffering in this place completely since it's not really a reliable mechanism, and add an opt-in mechanism if necessary. The problem as I see it is we don't know with certainty what scenarios rely on this buffering today.

I suggest the following plan:

1. Let this change go in (as a stop-gap for the OOM)
2. Add telemetry to track where we see listeners come in and buffer is not empty
3. Let it run for a week in Stable
4. With that data, remove buffering in fromService while opting in the places identified by telemetry
5. The previous step would revert this change and telemetry as obsolete

A way for others to check this themselves, on any build

Several people in this thread are reporting the crash but can only offer RSS or crash dumps, which is the hard way to tell "fixed" from "quiet afternoon". Here is a zero-dependency check that counts the leak's own signature instead:

https://gist.github.com/jeremyprz/0f4a353efaf8de58fb32fdc8d1ce7f9b

$ vscode-pty-leak-check.py main-A.heapsnapshot main-B.heapsnapshot
snapshot                          pty chunks   retained    strings
main-A.heapsnapshot                      322       4.8M     98,357
main-B.heapsnapshot                   27,219     430.5M    125,468

LEAKING - 27,219 raw pty chunks retained (430.5 MB) in the main process.

  growth across the pair: +26,897 chunks (+425.7 MB)

That is an unpatched 1.137.0 main, ten minutes apart, under ordinary use — +26,897 retained pty chunks, +425.7 MB. The same pair of snapshots taken after unbuffering onProcessData reports 0 and 0.

It counts strings ≥ 4 KB containing an ANSI escape — the raw pty chunks themselves — so an idle-terminal snapshot and a fixed build no longer look alike, and the output says so explicitly when the count is zero. Pure stdlib, takes .heapsnapshot files exported from DevTools, and the header explains how to get one off main (inspector applies at launch only; kill -USR1 terminates Electron rather than enabling it).

One thing I took out rather than shipped, since it bears on how much to trust any of this: it originally also looked for onProcessData among the snapshot's node names to confirm the retainer chain. On real snapshots that count came out identical whether the process was leaking or clean — what matches is the property name, which is present either way. A number that does not move with the thing being measured is worse than no number, so the chunk count is the only verdict it gives.

Might be useful for anyone who wants to confirm the fix on their own machine once this lands, or to tell whether a crash they are seeing is this bug or a different one.

I don't think adding more AI slop to the discussion on this PR is really helping anything. 😅

AFAICT this is just waiting for someone at MSFT (Anthony Kim (@anthonykim1) or Dmitriy Vasyura (@dmitrivMS)?) to look at it again.

Had anyone supplied a one-click workaround? There is now a one-click workaround for those who are impacted. Call it slop if you want - but it has mitigated my hourly annoyance that has plagued me for 1 year.

https://gist.github.com/jeremyprz/0f4a353efaf8de58fb32fdc8d1ce7f9b

Updated this in f63bfed7c4f to reuse unbufferedEvents instead of adding a whole-channel opt-out.

For local terminals, the renderer receives output and replay directly from the PTY host through PtyHostWindow. Main was also buffering those process events on LocalPty, but nothing subscribed to them there, so the buffers kept growing.

This only removes buffering for the unused process events. Host lifecycle and variable-resolution events stay buffered, and the renderer’s direct output/replay path is unchanged.

Tests) Added tests for selective buffering, restart notifications, and replay completion after renderer writes. The new reload and detach/attach smoke tests check that all 100 numbered output lines arrive exactly once and in order. Also verified terminal recovery after PTY-host and application restarts locally on macOS.

TLDR:

Uses unbufferedEvents for unused LocalPty process events: onProcessData, onProcessReady, onProcessExit, onProcessReplay, onDidChangeProperty, onProcessOrphanQuestion, and onDidRequestDetach.

Keeps buffering for host lifecycle and variable-resolution events: onPtyHostStart, onPtyHostExit, onPtyHostUnresponsive, onPtyHostResponsive, and onPtyHostRequestResolveVariables.

Edit: This should be available in next upcoming stable 1.138.0 and the next upcoming vscode insiders.
Will post here once we have such insiders ready for people to test on, thanks everyone.

The is now in latest VS Code Insiders.

FYI - the fix will be included in the VS Code 1.138.0 stable release.