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

Sandbox background process architecture #116337

Closed
bpasero opened this issue Feb 10, 2021 · 4 comments
Closed

Sandbox background process architecture #116337

bpasero opened this issue Feb 10, 2021 · 4 comments
Assignees
Labels
feature-request Request for new features or functionality sandbox Running VSCode in a node-free environment
Milestone

Comments

@bpasero
Copy link
Member

bpasero commented Feb 10, 2021

From discussion today in standup here are suggestions for the sandbox architecture:

  • leverage the existing shared process as much as possible (make it robust, start fast and have logging for crashes)
  • keep main process usage as little as possible
  • aggressively move things over to shared process (e.g. logger, sqlite?)
  • investigate into turning the shared process into a lightweight node.js process with MessagePort support and access to Electron's proxy solution for network requests (if we cannot get proxy support here, we could think about doing ad-hoc hidden windows for network requests as a workaround)

image

@bpasero bpasero added the sandbox Running VSCode in a node-free environment label Feb 10, 2021
@bpasero bpasero added this to the Backlog milestone Feb 10, 2021
@bpasero bpasero added the under-discussion Issue is under discussion for relevance, priority, approach label Feb 10, 2021
@Tyriar
Copy link
Member

Tyriar commented Feb 10, 2021

Won't this delay terminal creation for several seconds after start up? That was the main reason for keeping this out of Shared Process. Is this temporary based on whether we can make it node or when we get message port support would be move it to be a child of main?

@bpasero
Copy link
Member Author

bpasero commented Feb 11, 2021

@Tyriar once we let the shared process be the parent of the pty-host we will no longer delay startup. This can easily be controlled here:

this.lifecycleMainService.when(LifecycleMainPhase.AfterWindowOpen).then(() => {
this._register(new RunOnceScheduler(async () => {
sharedProcess.spawn(await resolveShellEnv(this.logService, this.environmentService.args, process.env));
}, 3000)).schedule();
});

@bpasero
Copy link
Member Author

bpasero commented Feb 17, 2021

After discussion in the Electron sync yesterday, here is a revised diagram given @deepak1556 feedback that the new process architecture will not provide MessagePort support for child processes spawned from the shared process. That means, all communication from the window will have to go through the shared process and then into the child processes:

image

@bpasero bpasero modified the milestones: Backlog, On Deck Apr 21, 2021
@bpasero bpasero added feature-request Request for new features or functionality and removed under-discussion Issue is under discussion for relevance, priority, approach labels Jun 15, 2021
@bpasero
Copy link
Member Author

bpasero commented Aug 25, 2021

Merged into #92164.

@bpasero bpasero closed this as completed Aug 25, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Oct 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality sandbox Running VSCode in a node-free environment
Projects
None yet
Development

No branches or pull requests

3 participants