Tweaks to the initial environment_{file|loader} implementation

- Remove the blocking call to subprocess.check_output in the main
supervisord process. Instead make that call in the child process
immediately after the initial fork, before doing any changes to the
process config and state. This keeps the blocking call out of the danger
zone, while still keeping the simple design.

Author: wynnw

Diff for: 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.output))
+                )
 
         if envdata:
             extra_env = {}

Diff for: 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,