Repository navigation
Fix main process OOM from unconsumed buffered pty host service events - #323980
Anthony Kim (anthonykim1) merged 9 commits into
Conversation
…#307156) ProxyChannel.fromService eagerly wraps every service event in Event.buffer, which subscribes immediately and retains every event until a first listener attaches. The LocalPty channel's events are never listened to on desktop (terminal data flows over the dedicated pty host connection), so every pty data chunk was retained in the main process heap for the lifetime of the process, eventually reaching V8's heap ceiling and crashing with OOM. Adds a disableEventBuffering option to ProxyChannel and applies it to the LocalPty channel registration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: Robo (@deepak1556)Matched files:
|
There was a problem hiding this comment.
Pull request overview
This PR addresses a main-process memory leak/OOM in the desktop terminal IPC path by allowing ProxyChannel.fromService to expose service events without eager Event.buffer(...) wrapping, and applying that behavior to the LocalPty channel where events are never listened to on desktop.
Changes:
- Add
disableEventBufferingtoProxyChannel.ICreateServiceChannelOptionsto avoid eager subscription + buffering of service events. - Use
disableEventBuffering: truewhen registering theTerminalIpcChannels.LocalPtychannel in the electron-main process. - Add IPC unit tests covering eager subscription by default vs. the new no-buffering mode.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/vs/code/electron-main/app.ts | Registers ILocalPtyService channel with event buffering disabled to prevent unbounded heap growth. |
| src/vs/base/parts/ipc/common/ipc.ts | Introduces disableEventBuffering option and routes event creation through it. |
| src/vs/base/parts/ipc/test/common/ipc.test.ts | Adds unit tests for default eager subscription vs. disabled buffering behavior. |
|
@microsoft-github-policy-service agree |
… test, tighten comments Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Previous take #285419 |
|
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:
|
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
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 |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Reuse unbufferedEvents for unused process events while preserving lifecycle notifications. Cover lazy subscriptions, restart notifications, direct replay completion, and ordered output across reload and reattachment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
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 Keeps buffering for host lifecycle and variable-resolution events: |
f955049
into
microsoft:main
|
Edit: This should be available in next upcoming stable |
|
The is now in latest VS Code Insiders. |
…microsoft#323980) * Fix main process OOM from unconsumed buffered pty host service events (microsoft#307156) ProxyChannel.fromService eagerly wraps every service event in Event.buffer, which subscribes immediately and retains every event until a first listener attaches. The LocalPty channel's events are never listened to on desktop (terminal data flows over the dedicated pty host connection), so every pty data chunk was retained in the main process heap for the lifetime of the process, eventually reaching V8's heap ceiling and crashing with OOM. Adds a disableEventBuffering option to ProxyChannel and applies it to the LocalPty channel registration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Address review feedback: assert buffered delivery in default-behavior test, tighten comments Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Use selective buffering for local PTY events and cover replay Reuse unbufferedEvents for unused process events while preserving lifecycle notifications. Cover lazy subscriptions, restart notifications, direct replay completion, and ordered output across reload and reattachment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Jacob Jove <jacobjove@Jacobs-MacBook-Pro.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Anthony Kim <anthonykim@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> (cherry picked from commit f955049)

Fixes #234393. The crash dumps, heap snapshots, and retainer chains referenced below are in #307156, which was closed as a duplicate of it.
Problem
The main process accumulates all terminal PTY output in the V8 heap until it hits the ~4 GB heap ceiling and OOM-aborts (
Reached heap limit). Heavy ANSI producers in the integrated terminal (tmux status/pane redraws, TUI apps) crash the main process within hours; median ~4.2h across the 99 crash dumps in #307156.Root cause
ProxyChannel.fromServiceeagerly wraps every service event inEvent.buffer(..., flushAfterTimeout=true), which subscribes to the source immediately and queues each event into an array that is only flushed (and released) when a first listener attaches to the channel event.The main process registers
ILocalPtyServicethis way (app.ts) as theTerminalIpcChannels.LocalPtychannel. On desktop, the renderer consumes terminal data over the dedicated pty host connection (PtyHostWindowMessagePort, seelocalTerminalBackend.ts); theLocalPtychannel itself is only used for two method calls (getLatency,getProfiles) plus a handful of low-frequency pty-host lifecycle events (onPtyHostStart/onPtyHostExit/onPtyHostUnresponsive/onPtyHostResponsive/onPtyHostRequestResolveVariables, consumed byBaseTerminalBackend) — it never gets a listener for the high-volumeonProcessDataevent. So the eager buffer foronProcessData(one entry per PTY data chunk) grows for the lifetime of the process. Retainer chains in heap snapshots (see #307156) show ~99% of the heap held by exactly this array:Emitter._listeners → closure → Array[N] → { event: }.Fix
Adds a
disableEventBufferingoption toProxyChannel.ICreateServiceChannelOptions(naming follows the existingdisableMarshalling) that exposes service events on the channel without the eagerEvent.bufferwrapper, and applies it to theLocalPtychannel registration. Events fired before a listener attaches are dropped instead of retained. This is a whole-channel opt-out, so it also stops buffering the pty-host lifecycle eventsBaseTerminalBackendlistens to here; in practice that's low-risk (those fire only after a window's listener has attached, and the one early-fireable case — the initialonPtyHostStart— is a no-op perBaseTerminalBackend's ownhasStartedguard, see review discussion below). Per Dmitriy Vasyura (@dmitrivMS)'s plan, this PR ships as the accepted stop-gap; a follow-up telemetry pass will confirm whether any channel actually needs buffering restored before removing it fromfromServicegenerally.Testing
ipc.test.tscover the default (eager subscription, buffered) and the new option (no subscription untillisten, events delivered after).--disable-extensions, macOS arm64), measuring main-process RSS every 15 s on the same dev build with only this change toggled: