Skip to content

Commit

Permalink
Remove daemonize method + tests (#10749)
Browse files Browse the repository at this point in the history
`ProcessManager` has a legacy `daemonize` method that used to be used to spawn pantsd, but no longer is called by anything. This commit removes the method and several test cases that exercised it.
  • Loading branch information
gshuflin committed Sep 9, 2020
1 parent ef92e48 commit 9a97dee
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 87 deletions.
75 changes: 3 additions & 72 deletions src/python/pants/pantsd/process_manager.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# Copyright 2015 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

import functools
import logging
import os
import signal
Expand Down Expand Up @@ -526,83 +525,15 @@ def terminate(self, signal_chain=KILL_CHAIN, kill_wait=KILL_WAIT_SEC, purge=True
if purge:
self.purge_metadata(force=True)

def daemonize(
self,
pre_fork_opts=None,
post_fork_parent_opts=None,
post_fork_child_opts=None,
fork_context=None,
write_pid=True,
):
"""Perform a double-fork, execute callbacks and write the child pid file.
The double-fork here is necessary to truly daemonize the subprocess such that it can never
take control of a tty. The initial fork and setsid() creates a new, isolated process group
and also makes the first child a session leader (which can still acquire a tty). By forking a
second time, we ensure that the second child can never acquire a controlling terminal because
it's no longer a session leader - but it now has its own separate process group.
Additionally, a normal daemon implementation would typically perform an os.umask(0) to reset
the processes file mode creation mask post-fork. We do not do this here (and in daemon_spawn
below) due to the fact that the daemons that pants would run are typically personal user
daemons. Having a disparate umask from pre-vs-post fork causes files written in each phase to
differ in their permissions without good reason - in this case, we want to inherit the umask.
:param fork_context: A function which accepts and calls a function that will call fork. This
is not a contextmanager/generator because that would make interacting with native code more
challenging. If no fork_context is passed, the fork function is called directly.
"""

def double_fork():
logger.debug("forking %s", self)
pid = os.fork()
if pid == 0:
os.setsid()
second_pid = os.fork()
if second_pid == 0:
return False, True
else:
if write_pid:
self.write_pid(second_pid)
return False, False
else:
# This prevents un-reaped, throw-away parent processes from lingering in the process table.
os.waitpid(pid, 0)
return True, False

fork_func = functools.partial(fork_context, double_fork) if fork_context else double_fork

# Perform the double fork (optionally under the fork_context). Three outcomes are possible after
# the double fork: we're either the original parent process, the middle double-fork process, or
# the child. We assert below that a process is not somehow both the parent and the child.
self.purge_metadata()
self.pre_fork(**pre_fork_opts or {})
is_parent, is_child = fork_func()

try:
if not is_parent and not is_child:
# Middle process.
os._exit(0)
elif is_parent:
assert not is_child
self.post_fork_parent(**post_fork_parent_opts or {})
else:
assert not is_parent
os.chdir(self._buildroot)
self.post_fork_child(**post_fork_child_opts or {})
except Exception:
logger.critical(traceback.format_exc())
os._exit(0)

def daemon_spawn(
self, pre_fork_opts=None, post_fork_parent_opts=None, post_fork_child_opts=None
):
"""Perform a single-fork to run a subprocess and write the child pid file.
Use this if your post_fork_child block invokes a subprocess via subprocess.Popen(). In this
case, a second fork such as used in daemonize() is extraneous given that Popen() also forks.
Using this daemonization method vs daemonize() leaves the responsibility of writing the pid
to the caller to allow for library-agnostic flexibility in subprocess execution.
case, a second fork is extraneous given that Popen() also forks. Using this daemonization
method leaves the responsibility of writing the pid to the caller to allow for library-
agnostic flexibility in subprocess execution.
"""
self.purge_metadata()
self.pre_fork(**pre_fork_opts or {})
Expand Down
15 changes: 0 additions & 15 deletions tests/python/pants_test/pantsd/test_process_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -475,21 +475,6 @@ def mock_daemonize_context(self, chk_pre=True, chk_post_child=False, chk_post_pa
if chk_post_parent:
mock_post_parent.assert_called_once_with(self.pm)

def test_daemonize_parent(self):
with self.mock_daemonize_context() as mock_fork:
mock_fork.side_effect = [1, 1] # Simulate the parent.
self.pm.daemonize(write_pid=False)

def test_daemonize_child(self):
with self.mock_daemonize_context(chk_post_child=True) as mock_fork:
mock_fork.side_effect = [0, 0] # Simulate the child.
self.pm.daemonize(write_pid=False)

def test_daemonize_child_parent(self):
with self.mock_daemonize_context(chk_post_parent=True) as mock_fork:
mock_fork.side_effect = [1, 1] # Simulate the original parent process.
self.pm.daemonize(write_pid=False)

def test_daemon_spawn_parent(self):
with self.mock_daemonize_context(chk_post_parent=True) as mock_fork:
mock_fork.return_value = 1 # Simulate the parent.
Expand Down

0 comments on commit 9a97dee

Please sign in to comment.