Fix parent process being killed on Windows when checking if it's alive #445
Conversation
Fixes issue #434 The `os.kill()` function being used to check for the existence of the parent process causes the owning process to be killed on Windows platforms. This commit therefore introduces an alternative solution to check for the parent process to be alive by using the Windows API types.windll.kernel32.OpenProcess() An alternative would have been to introduce psutil library as another dependency, which was judged not being worth.
|
ping @gatesn @jiahuijiang ? |
|
I think this was wrongly closed. |
|
Right now if you run python language server without passing a --check-parent-process flag it will not check the parent process aliveness anymore |
|
... but if you do so on Windows, the parent process is still killed due to invalid usage of This flag just asks the user to actively trigger the still existing bug, by passing the flag. |
|
Yeah, that's why I think this should be reopened. |
|
This fix is critical on Windows; its apparently leading to many instances of zombie PyLS processes sticking around and taking up substantial memory and resources until the system is rebooted or the user knows to manually find and kill them. This and other stalled/rejected patches has forced us to maintain our own downstream fork, but we'd like to see these important fixes merged upstream to benefit all your users as well. Thanks! |
@gatesn, he's talking about Spyder (@CAM-Gerlach is one of our developers). We haven't decided yet if we would have to create a fork of this project for Spyder, but we will certainly have to if things like this PR are finally rejected. Could you consider reviewing your decision? Thanks! |
|
@andfoy, please create another PR based on this one and resubmit it to this repo so we can merge it. This basically makes the parent process check mechanism work on Windows. |
|
@deathaxe, we have to that because tests are not passing here. |
|
@deathaxe, or if you're still interested in merging this PR, please rebase it. |
ccordoba12
changed the title
|
Thanks a lot @deathaxe for your contribution! |
ccordoba12
changed the title
Fixes #434
The
os.kill()function being used to check for the existence of the parent process causes the owning process to be killed on Windows platforms.This commit therefore introduces an alternative solution to check for the parent process to be alive by using the Windows API
types.windll.kernel32.OpenProcess()An alternative would have been to introduce psutil library as another dependency, which was judged not being worth.