-
Notifications
You must be signed in to change notification settings - Fork 34.5k
Closed
Labels
feature-requestRequest for new features or functionalityRequest for new features or functionalityfreeze-slow-crash-leakVS Code crashing, performance, freeze and memory leak issuesVS Code crashing, performance, freeze and memory leak issuesinsiders-releasedPatch has been released in VS Code InsidersPatch has been released in VS Code Insiderson-testplanperfterminalGeneral terminal issues that don't fall under another labelGeneral terminal issues that don't fall under another label
Milestone
Description
As of around a year ago node-pty is launched directly from the renderer process, while this does mean ctrl+c is responsive it has several downsides:
- node-pty crashes will take down the entire window (Often getting full window hangs #71966, Closing integrated terminal causes window to crash with C++ assertion error #71789, Crashes when quickly press ctrl+` twice #74181)
- Data events from node-pty seems to be blocked by the event loop, I suspect this is main the reason conpty is slow, it may also be slowing down non-Windows data flow too
- The remote case the connection can get overwhelmed by fast producing programs.
- node-pty can freeze the window when a busy loop is run in the terminal Freeze when huge data in terminal #106391
My proposal for these problems:
- Reintroduce terminalProcess to host node-pty, calling it nodePtyHostProcess is probably a better name, this protects the extension host and renderer from crashes
- Batch data events inside nodePtyHostProcess (Experiment with buffering the pty data to improve performance #54093), reducing the amount of total events in favor of larger ones
- Introduce a flow control mechanism which ensures the pty does not get too far ahead of xterm.js. This is a little more tricky for the remote case, one idea is to send an ack every x bytes from the renderer and nodePtyHostProcess will pause if it hasn't received the yth last ack (x and y need to be experimented with and may depend on latency) (related custom flow control and discard limit xtermjs/xterm.js#2122, flow control handling node-pty#304)
SupinePandora43, schickling, gitkalz, usernamehw, heartacker and 5 more
Metadata
Metadata
Assignees
Labels
feature-requestRequest for new features or functionalityRequest for new features or functionalityfreeze-slow-crash-leakVS Code crashing, performance, freeze and memory leak issuesVS Code crashing, performance, freeze and memory leak issuesinsiders-releasedPatch has been released in VS Code InsidersPatch has been released in VS Code Insiderson-testplanperfterminalGeneral terminal issues that don't fall under another labelGeneral terminal issues that don't fall under another label