GH-835: Made sure forked process is terminated on application quit. #836
Conversation
kittaakos
changed the title
| // If we forked the process for the clusters, we need to manually terminate it. | ||
| // See: https://github.com/theia-ide/theia/issues/835 | ||
| process.kill(cp.pid); | ||
| }) |
There was a problem hiding this comment.
I missed the semicolon here. :(
Adjusted server worker log messages, logged PID too. Signed-off-by: Akos Kitta <kittaakos@gmail.com>
|
I have verified the changes on OS X with some trick from here. It fixes the issue. The application works, but it does not fix anything on Windows. The LS process is a different store. I still need to verify one more use case on Ubuntu. |
I meant: "story". |
|
Something is odd. I can select a WS root, but then I cannot change. I do not know whether it is related to my change or not. I will update this thread. |
|
| @@ -120,6 +120,11 @@ if (cluster.isMaster) { | |||
| console.error(error); | |||
| electron.app.exit(1); | |||
| }); | |||
| electron.app.on('quit', function() { | |||
| // If we forked the process for the clusters, we need to manually terminate it. | |||
There was a problem hiding this comment.
I wonder is it the same when cluster forks, should we clean up for them too? Right now electron process -> master process -> worker process -> ls process. This change kills the master process. But if the worker process is still running, when the ls process won’t auto shutdown itself according the lsp.
There was a problem hiding this comment.
I think so. I had the same feeling. But I would like to tackle that as part of #815. I plan to proceed with smaller changes and verify them one by one on each platform with the bundled electron application.
There was a problem hiding this comment.
My point is that probably nothing should be done about ls servers, only workers should be killed, and ls processes will exit according to the lsp.
There was a problem hiding this comment.
OK. We'll see how that works in practice, especially on Windows in the context of #815.
Adjusted server worker log messages, logged PID too.
Closes #835.
Signed-off-by: Akos Kitta kittaakos@gmail.com