diff --git a/supervisor/options.py b/supervisor/options.py index 3dc84afd9..fae938e02 100644 --- a/supervisor/options.py +++ b/supervisor/options.py @@ -1955,7 +1955,8 @@ def make_dispatchers(self, proc): def load_external_environment_definition(self): return self.load_external_environment_definition_for_config(self) - # this is separated out in order to make it easier to test + # NOTE - THIS IS BLOCKING CODE AND MUST ONLY BE CALLED IN TESTS OR IN CHILD PROCESSES, NOT THE + # MAIN SUPERVISORD THREAD OF EXECUTION @classmethod def load_external_environment_definition_for_config(cls, config): # lazily load extra env vars before we drop privileges so that this can be used to load a secrets file @@ -1976,21 +1977,15 @@ def load_external_environment_definition_for_config(cls, config): elif config.environment_loader: try: - from subprocess import check_output, CalledProcessError - kwargs = dict(shell=True) - if sys.version_info.major >= 3: - if sys.version_info.minor >= 7: - kwargs['text'] = True - else: - pass # we will decode the bytes returned after reading it for these versions of python - - envdata = check_output(config.environment_loader, **kwargs) + from subprocess import check_output, CalledProcessError, STDOUT as subprocess_STDOUT - if sys.version_info.major >= 3 and sys.version_info.minor < 7: - envdata = envdata.decode('utf8') + envdata = check_output(config.environment_loader, shell=True, stderr=subprocess_STDOUT) + envdata = as_string(envdata) except CalledProcessError as e: - raise ProcessException("environment_loader failure with %s: %d, %s" % (config.environment_loader, e.returncode, e.output)) + raise ProcessException("environment_loader failure with %s: %d, %s" % \ + (config.environment_loader, e.returncode, as_string(e.stdout)) + ) if envdata: extra_env = {} diff --git a/supervisor/process.py b/supervisor/process.py index b047886e5..e303a6b2d 100644 --- a/supervisor/process.py +++ b/supervisor/process.py @@ -218,9 +218,6 @@ def spawn(self): try: filename, argv = self.get_execv_args() - # check the environment_file/environment_loader options before we fork to simplify child process management - extra_env = self.config.load_external_environment_definition() - except ProcessException as what: self.record_spawnerr(what.args[0]) self._assertInState(ProcessStates.STARTING) @@ -264,7 +261,7 @@ def spawn(self): return self._spawn_as_parent(pid) else: - return self._spawn_as_child(filename, argv, extra_env=extra_env) + return self._spawn_as_child(filename, argv) def _spawn_as_parent(self, pid): # Parent @@ -288,9 +285,17 @@ def _prepare_child_fds(self): for i in range(3, options.minfds): options.close_fd(i) - def _spawn_as_child(self, filename, argv, extra_env=None): + def _spawn_as_child(self, filename, argv): options = self.config.options try: + # check the environment_file/environment_loader options after forking in order to avoid blocking the + # main supervisord thread, but do it before we start to mix up the process signals/state + try: + extra_env = self.config.load_external_environment_definition() + except ProcessException as e: + self.record_spawnerr(e.args[0]) + raise + # prevent child from receiving signals sent to the # parent by calling os.setpgrp to create a new process # group for the child; this prevents, for instance,