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.

Sign up for GitHub

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

Jump to bottom

[windows] Spawned processes leak on Windows 10 #815

Closed

kittaakos opened this issue Nov 10, 2017 · 17 comments

Labels

OS/Windows

Contributor

kittaakos commented Nov 10, 2017

Start a bundled electron application, kill the application, one can still see stale processes on Windows.

See: theia-ide/yangster-electron#8
Also: microsoft/vscode#26807

kittaakos self-assigned this

kittaakos added a commit that referenced this issue


          GH-815: Bumped up the node-pty version.

The new version is intentionally 0.7.1 to be sync with VSCode.

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

hexa00 commented Nov 10, 2017

Is the node/electron process gone however ?

Makes me think of the backend not stopping on windows after ctrl-c

Contributor Author

kittaakos commented Nov 10, 2017

Neither 0.7.1 nor 0.7.2 fixes the problem in the bundled electron application.

Contributor Author

kittaakos commented Nov 10, 2017

Contributor Author

kittaakos commented Nov 10, 2017

PR: #816.

kittaakos mentioned this issue

GH-815: Bumped up the node-pty version. #816

Closed

Contributor Author

kittaakos commented Nov 10, 2017

I try with ps-tree.

hexa00 commented Nov 10, 2017

But we're not using node-pty to create a LSP process ?

How is this related?

Contributor Author

kittaakos commented Nov 10, 2017

I could ask the same, why do you want me to update node-pty if that does not fix my issue?

hexa00 commented Nov 10, 2017

@kittaakos I do not ? I said that because of #816

I think like I said in #816 you should use RawProcess that may fix it...

Contributor Author

kittaakos commented Nov 10, 2017

Let's give it a try. Thanks for #817.

kittaakos mentioned this issue

GH-835: Made sure forked process is terminated on application quit. #836

Merged

kittaakos added a commit that referenced this issue


          GH-815: Simplified the process API a bit.

f303e97

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Exposed writable stream process API.

fe10601

Replaced `child_process` with `RawProcess` in LS contribution.

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos mentioned this issue

GH-815: Replace child_process with the Process API in the LS backend contribution #841

Merged

kittaakos added a commit that referenced this issue


          GH-815: Added the missing process dependency.

b4a3d07

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Exposed writable stream process API.

33c7342

Replaced `child_process` with `RawProcess` in LS contribution.

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Initial changes to support LS process termination.

d235fe1

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos mentioned this issue

[Windows] I can see multiple stale Node.js processes after terminating the application #633

Closed

kittaakos added a commit that referenced this issue


          GH-815: Fixed uninitialized process ID array.

f0c97cb

[skip ci]

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Simplified the process API a bit.

58aea94

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Exposed writable stream process API.

07eb061

Replaced `child_process` with `RawProcess` in LS contribution.

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Initial changes to support LS process termination.

1ac8d3a

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Fixed process ID generation. Replaced type with interface.

d44bd12

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Fixed incorrect signal even when terminating process.

5f4a5df

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Simplified the process API a bit.

799424c

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Exposed writable stream process API.

ed43fad

Replaced `child_process` with `RawProcess` in LS contribution.

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Initial changes to support LS process termination.

04157f6

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Fixed process ID generation. Replaced type with interface.

c5ad20c

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Initial changes to support LS process termination.

ab689d0

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Fixed process ID generation. Replaced type with interface.

ad7deea

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Fixed incorrect signal event when terminating process.

e45dfa5

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Replaced process ID array with disposables.

580dad7

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Made process shutdown a bit more generic.

1e060a3

Instead of terminating all the LS contribution processes, we shut down all processes spawned from the process manager.

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Made sure process is automatically unregsitered.

On both `exit` and `error`.

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Removed terminal writeable stream.

b5b9e0b

Also changed the info log level to debug in the process manager.

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added the OS/Windows label

kittaakos closed this as completed in #841

kittaakos added a commit that referenced this issue


          GH-815: Simplified the process API a bit.

79ecbdb

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Exposed writable stream process API.

9b19551

Replaced `child_process` with `RawProcess` in LS contribution.

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Initial changes to support LS process termination.

8fe8ecb

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Fixed process ID generation. Replaced type with interface.

e274c62

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Fixed incorrect signal event when terminating process.

9aa1ecd

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Replaced process ID array with disposables.

646b121

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Made process shutdown a bit more generic.

03b9c74

Instead of terminating all the LS contribution processes, we shut down all processes spawned from the process manager.

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Made sure process is automatically unregsitered.

998a5c2

On both `exit` and `error`.

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos added a commit that referenced this issue


          GH-815: Removed terminal writeable stream.

6601c8f

Also changed the info log level to debug in the process manager.

Signed-off-by: Akos Kitta <kittaakos@gmail.com>

kittaakos reopened this

Contributor Author

kittaakos commented Dec 4, 2017

The problem still happens on Windows with a bundled application.

Member

akosyakov commented Dec 4, 2017

Which process? YANG LS?

Contributor Author

kittaakos commented Dec 4, 2017

Yes, the LS process.

hexa00 commented Dec 12, 2017

I think this is this issue: microsoft/node-pty#7

Note it doesn't happen if you use node directly it only happens when you use yarn start.

Maybe we could work around with a script to start it for now with node ?

hexa00 mentioned this issue

Fix backend exit and cleanup #972

Merged

Tyriar commented Dec 14, 2017

microsoft/node-pty#7 is about pressing ctrl+c either not closing an application or corrupting the viewport in some way.

It's not totally clear to me what the problem is, but if it's about processes leaking under node-pty like microsoft/vscode#26807, then #816 should have fixed this issue.

See the release notes for 0.7.1:

All console processes running inside the pty should now be killed when killing the pty. This issue was observed frequently when running web servers via node-pty #122 via @the-ress, @Tyriar

hexa00 commented Dec 14, 2017

ok I have the same issue as described in microsoft/node-pty#7 (comment)

But I'm not running inside node-pty it's directly git bash.

Maybe git bash needs a simliar fix as: rprichard/winpty#130 from winpty

Tyriar commented Dec 15, 2017

Yeah that may be the case then

kittaakos removed their assignment

Contributor Author

kittaakos commented Mar 7, 2019

I do not see this anymore. I have verified it on my Windows 10 image with various Xtext based language servers.

I can confirm, the LS (java) process termination was successful. However, the process termination is not instantaneous on Windows. It is because the child processes do not die automatically on Windows when the parent process stops. We have to poll from the LS.

I am closing this.

kittaakos closed this as completed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment