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

Move terminal process to main #116185

Closed
wants to merge 11 commits into from
Closed

Move terminal process to main #116185

wants to merge 11 commits into from

Conversation

Tyriar
Copy link
Member

@Tyriar Tyriar commented Feb 9, 2021

Part of #74620
Fixes #71966

High level changes:

  • Move local terminal process creation into electron-main - this is expected to improve terminal performance on Windows significantly
  • Create TerminalProcessMainProxy that implements the ITerminalChildProcess interface across IPC, this behaves similar to TerminalProcessExtHostProxy
  • Move parts of terminal into platform as required
  • Updated node-pty to include the hang fix on Windows Host conout socket in a worker node-pty#415, Often getting full window hangs #71966

Next steps:

  • Move the terminal process into a ptyhost process off of main:
    Main
      ptyhost
        bash
      Renderer
    
    ptyhost will be a regular node process such that we have access to worker_threads so we can fix the hang issue on Windows (Often getting full window hangs #71966).
  • When support in Electron lands, use message ports between the pty host and the renderer such that main doesn't need to deal with the potential busy terminal data events, it's expected that we will handle the file watcher in a similar way where the native module is hosted in it's own process off main.

@Tyriar Tyriar added this to the February 2021 milestone Feb 9, 2021
@Tyriar Tyriar self-assigned this Feb 9, 2021
This brings in microsoft/node-pty#415 which will finally fix the hang issue
on Windows with conpty. Performance on Windows should also improve a lot
because node-pty is now hosted outside of the cluttered renderer process.

Fixes #71966
@Tyriar
Copy link
Member Author

Tyriar commented Feb 10, 2021

Closing in favor of #116373, we won't merge this in until the pty lives outside of main (context: #116337)

@Tyriar Tyriar closed this Feb 10, 2021
@Tyriar Tyriar deleted the tyriar/74620_2 branch February 22, 2021 12:51
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Often getting full window hangs
1 participant