Move local terminal processes to live under the new ptyHost process #116373

Tyriar · 2021-02-10T20:08:06Z

More descriptive as it's for local/electron terminal processes only

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

Move local ptys to ptyHost under shared process

This reverts commit 8912ec1.

This reverts commit 4713b13.

This reverts commit dc4fa48.

Tyriar · 2021-02-16T19:08:50Z

Think I finally figured out the problem now that I can log:

The terminal process failed to launch: A native exception occurred during launch (Cannot find module '/__w/1/VSCode-linux-x64/resources/app/node_modules.asar/node-pty/lib/index.js'. Please verify that the package.json has a valid "main" entry).

Worker threads don't appear to play nicely with asar which was causing only the bundled version of VS Code's terminal to break

Tyriar · 2021-02-18T12:47:06Z

Unfortunately this is going to be merged without the fix to #76548, the problem I've been stuck on for a few days was caused once again by worker_threads but this time it failed only within our packages build in Azure DevOps because that tests with our product build that has extra optimization steps which includes packaging node modules via asar.

It seems that asar simply does not support resolving lookups in workers based on my own findings and from some searching (eg). So I need to make a minimal repro and create an issue in Electron. There is asar-node seemingly to solve this problem but I don't think it has the kind of usage we need, it does monkey patching to accomplish its goals 😱 and it's an unexpectedly large amount of code.

Tyriar and others added 12 commits February 8, 2021 12:16

Move remoteterminalservice reg to contribution

3021226

Create pty service main <-> renderer channel

ee4a8bd

Part of #74620

Rename pty service to local pty service

281d228

More descriptive as it's for local/electron terminal processes only

Create createProcess ipc interface

456841a

Pass executableEnv to other side

1e298d0

Get one way communication happening

6132a76

Get 2 way communication working

47adc86

Remove log

4b5108a

Merge remote-tracking branch 'origin/master' into tyriar/74620_2

3a3b42a

Merge remote-tracking branch 'origin/master' into tyriar/74620_2

f2f9d1b

node-pty@0.11.0-beta1

7b64e24

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

Merge remote-tracking branch 'origin/master' into tyriar/74620_2

1abb2d9

Tyriar added this to the February 2021 milestone Feb 10, 2021

Tyriar assigned meganrogge Feb 10, 2021

Tyriar marked this pull request as draft February 10, 2021 20:08

Tyriar self-assigned this Feb 10, 2021

Tyriar mentioned this pull request Feb 10, 2021

Move terminal process to main #116185

Closed

Tyriar and others added 6 commits February 10, 2021 14:03

Move pty to shared process

67dda13

Create ptyHost, bidi communication

25c680c

Clean up

6bdc6bb

Remove unneeded file, document channels

4ea18e9

Move ptyHostMain into the node folder

5188792

Merge pull request #116389 from microsoft/tyriar_megan_sharedproc

1819dd5

Move local ptys to ptyHost under shared process

This was referenced Feb 11, 2021

Often getting full window hangs #71966

Closed

Revisit shared process and main process services #114418

Closed

Tyriar force-pushed the tyriar/megan branch from 1701859 to 1819dd5 Compare February 11, 2021 12:26

Tyriar added 4 commits February 11, 2021 05:14

ILocalPtyService -> IPtyService

5fc19f5

Merge remote-tracking branch 'origin/master' into tyriar/megan

59264e2

Handle ptyHost exit gracefully and restart

1b72bb6

TerminalProcessMainProxy -> LocalPty

5de7be2

Tyriar added 7 commits February 12, 2021 15:20

Try conpty in integration tests

6841c52

Revert "Move ptyService out of shared proc temporarily"

1b89736

This reverts commit 8912ec1.

Revert "Register ptyservice in localptyservice"

656c907

This reverts commit 4713b13.

Revert "Bring ptyService into shared proc"

3d9a48e

This reverts commit dc4fa48.

Remove ps tree logging

8853436

Simplify echo works in the default shell

0e16b7a

Skip echo test again

90f344f

joaomoreno changed the base branch from master to main February 15, 2021 08:51

Tyriar added 4 commits February 16, 2021 02:44

Merge remote-tracking branch 'origin/main' into tyriar/megan

982b28f

Merge remote-tracking branch 'origin/main' into tyriar/megan

3859ba9

Log all lines in failing test

35abaaf

Log more in test

994ad37

Tyriar added 9 commits February 16, 2021 11:09

Include lib in node-pty

e091e89

Recursively include lib

939038a

Allow conoutSocketWorker in unpacked asar

14923b5

Re-enable all tests

c41fb76

Remove bundling changes

6f23480

Revert back to old node-pty

cb95415

Worker threads don't appear to play nicely with asar which was causing only the bundled version of VS Code's terminal to break

Remove logs from test

16510df

Re-enable pty host restarting

a6cc654

Merge remote-tracking branch 'origin/main' into tyriar/megan

666351b

Tyriar mentioned this pull request Feb 18, 2021

Host conout socket in a worker microsoft/node-pty#415

Merged

Update distro

da00865

Tyriar merged commit fdaebc1 into main Feb 18, 2021

Tyriar deleted the tyriar/megan branch February 18, 2021 13:35

Tyriar mentioned this pull request Feb 18, 2021

Handle non-responsive pty host processes #116948

Closed

IllusionMH mentioned this pull request Feb 24, 2021

Closing a conpty terminal hangs Code #117599

Closed

github-actions bot locked and limited conversation to collaborators Apr 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move local terminal processes to live under the new ptyHost process #116373

Move local terminal processes to live under the new ptyHost process #116373

Tyriar commented Feb 10, 2021 •

edited

Loading

Tyriar commented Feb 16, 2021

Tyriar commented Feb 18, 2021

Move local terminal processes to live under the new ptyHost process #116373

Move local terminal processes to live under the new ptyHost process #116373

Conversation

Tyriar commented Feb 10, 2021 • edited Loading

Tyriar commented Feb 16, 2021

Tyriar commented Feb 18, 2021

Tyriar commented Feb 10, 2021 •

edited

Loading