Skip to content

Fix proxycommand leaks#2019

Merged
sphuber merged 4 commits into
aiidateam:developfrom
greschd:fix_proxycommand_leaks
Oct 4, 2018
Merged

Fix proxycommand leaks#2019
sphuber merged 4 commits into
aiidateam:developfrom
greschd:fix_proxycommand_leaks

Conversation

@greschd
Copy link
Copy Markdown
Member

@greschd greschd commented Oct 4, 2018

Fixes #2018 and fixes #1923 and fixes #1853

This addresses two issues:

  • The constructor of _DetachedProxyCommand should not call super. The reason for this is that the detached version creates a different subprocess. If the super is also called, two subprocesses are created.
  • In the close method of _DetachedProxyCommand, the subprocess is killed more aggressively, and we poll the subprocess to avoid having defunct processes.

The subprocess killing is implemented as follows:

  • .terminate() the subprocess
  • .poll() for 2 seconds to see if it exits
  • if it hasn't, .kill(), and .poll() again for 2 seconds

This part is a bit hand-wavy, but I wanted to avoid using the .wait() method. On balance I think it's better if the daemon just continues if the subprocess is really unkillable.

@greschd greschd requested a review from sphuber October 4, 2018 09:47
@coveralls
Copy link
Copy Markdown

coveralls commented Oct 4, 2018

Coverage Status

Coverage increased (+4.5%) to 67.692% when pulling 613e2a7 on greschd:fix_proxycommand_leaks into f9a5624 on aiidateam:develop.

Copy link
Copy Markdown
Contributor

@sphuber sphuber left a comment

Choose a reason for hiding this comment

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

Thanks @greschd

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

Labels

None yet

Projects

None yet

3 participants