Prevent nailgun on nailgun violence when using symlinked java paths
Review Request #2538 - Created July 28, 2015 and submitted
|areitz, benjyw, jsirois, nhoward_tw, stuhood, zundel|
- Improve ProcessManager & NailgunExecutor properties - Address a bug (described in the PR) that causes nailguns to kill each other when java paths are symlinked - Improve logging for all cases under which the nailgun can be restarted
repro'd + tested in production CI verification job.
I feel like the None check should be here, since this is the place where it might be introduced, ie
cmdline = getattr(...) if cmdline: self._parse...(...)
Also, you could use
self.cmdlinehere instead of getattr.
I feel like having a debug statement about spawning is still useful. You could move it to
_get_nailgun_client, so that it is at the same level as the other, related, debug log.
I'm coming into this a little late, but why do we still want to keep both
ProcessManager.cmd()? It seems like we could pick one form to be the canonical way that pants represents an in-memory process, and convert all of the other usages to the canonical form.
This LGTM, but it would be nice to get a test that setup a symlinked java distribution from a real located java distribution to confirm and keep this fix alive.
Going forward its slightly more useful to put the PR ID in the bugs field (
rbt --bugs 1877 ...or via web ui).
Neither Distribution nor the cmd property here is explicit about the type of path it returns. Its seems to me os.path.realpath'ing in this code or pushing the guarantees down would be a good idea to both make the potential trap more clear, and bulletproff against api and/or platform differences.
In the current psutil doc - newer than our version of course - there is no mention that name or cmdline can be unavailable, just general talk about pid-cycling potentially leading to getting wrong answers to various process questions. Is all this dancing and downstream None checking it forces needed?