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

fix: ensure analytics and cleanup processes are not leaking #4599

Merged
merged 1 commit into from
May 8, 2019

Conversation

rosen-vladimirov
Copy link
Contributor

When CLI dies, the analyticsBroker and cleanupProcess processes receive disconnect event. At this point they have to finish their work and die silently after that.
However, this does not happen for analyticsBroker process as in the disconnect handler we call process.disconnect method. It fails, as the process is already disconnected. So the process hangs in undefined state and it leaks. The stareted cleanup process from it also leaks.
To fix this remove the disconnect call from the handler.
Also ensure injector is disposed before calling process.exit in the detached process's handler.

PR Checklist

What is the current behavior?

What is the new behavior?

Fixes issue #4593

When CLI dies, the `analyticsBroker` and `cleanupProcess` processes receive disconnect event. At this point they have to finish their work and die silently after that.
However, this does not happen for `analyticsBroker` process as in the `disconnect` handler we call `process.disconnect` method. It fails, as the process is already disconnected. So the process hangs in undefined state and it leaks. The stareted cleanup process from it also leaks.
To fix this remove the disconnect call from the handler.
Also ensure injector is disposed before calling process.exit in the detached process's handler.
@rosen-vladimirov rosen-vladimirov added this to the 5.4.0 milestone May 8, 2019
@rosen-vladimirov rosen-vladimirov self-assigned this May 8, 2019
@cla-bot cla-bot bot added the cla: yes label May 8, 2019
@ghost ghost added the new PR label May 8, 2019
@rosen-vladimirov rosen-vladimirov merged commit 3a3d3bf into master May 8, 2019
@rosen-vladimirov rosen-vladimirov deleted the vladimirov/fix-leaking-processes branch May 8, 2019 15:28
@ghost ghost removed the new PR label May 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants