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

Kill the subprocess when the client/server is dead #34

Merged
merged 2 commits into from
Feb 18, 2025

Conversation

mtshiba
Copy link
Contributor

@mtshiba mtshiba commented Feb 17, 2025

Hello, thanks for the great work.

When I was editing a (relatively large) Rust project on VSCode for MacOS with the rustowl extension enabled, I noticed that the computer would hang due to an abnormal CPU load. In the current language server implementation, it seems that re-analyzation is performed every time the code is changed, but it did not seem to abort the previous unfinished task/command at that time (just shutting down join will not kill child), so I improved it so that it aborts them.

Also, if an editor or language server crashes for some reason, rustowlc seems to remain as an orphan process, so I changed it so that cargo owl/rustowlc is also killed when the editor or owlsp dies.

I also fixed a typo.

Please feel free to let me know if there is any issue with the modified code.


こんにちは。興味深くプロジェクトを拝見させていただいております。

MacOSのVSCode上でRustOwl拡張機能を有効化したまま(比較的大きな)プロジェクトを編集していたところ、異常なCPU負荷でコンピューターがハングしてしまう事象を確認しました。現在のランゲージサーバーの実装ではコードの変更があるたびに再検査を行なっているようですが、その際に前回の未完了のタスクとコマンドをabortしていないようでしたので(joinをshutdownしただけではchildはkillされません)、abortするように改良しました。

また、何らかの原因でエディタないしランゲージサーバーがクラッシュした場合rustowlcが孤立プロセスとして残留してしまうようでしたので、エディタもしくはowlspが死んだ場合cargo owl/rustowlcもまとめてkillするようにしました。

また、typoの修正も行いました。

変更したコードに何か問題があれば遠慮なくお申し付けください。

@cordx56
Copy link
Owner

cordx56 commented Feb 17, 2025

Thank you so much! I was aware of some issues related to process management, so your PR is very helpful!

I’m going to sleep now, so I’ll review your PR after I wake up. Sorry for the delay!

いつも活動を拝見させていただいております。非常に助かります。レビューまで少々お待ちください。

Copy link
Collaborator

@chansuke chansuke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for your work!! I left some comments.

Copy link
Owner

@cordx56 cordx56 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mtshiba Please check my comments and let me know your opinion.

@mtshiba
Copy link
Contributor Author

mtshiba commented Feb 18, 2025

I think the suggestions are reasonable.
I've improved error handling according to the suggested review comments.

Copy link
Owner

@cordx56 cordx56 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I reviewed it, and it looks good. I appreciate your great work.

@cordx56 cordx56 merged commit 56c6bd2 into cordx56:main Feb 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants