Rollback 1ac6e37

gpshead · gpshead · commit bbe32966c15f · 2022-01-24T08:33:01.000Z
bpo-39207: Spawn workers on demand in ProcessPoolExecutor (pythonGH-19453) was implemented in a way that introduced child process deadlocks in some environments due to mixing threading and fork() in the parent process.
diff --git a/Doc/whatsnew/3.9.rst b/Doc/whatsnew/3.9.rst
@@ -404,6 +404,8 @@ Workers in :class:`~concurrent.futures.ProcessPoolExecutor` are now spawned on
 demand, only when there are no available idle workers to reuse. This optimizes
 startup overhead and reduces the amount of lost CPU time to idle workers.
 (Contributed by Kyle Stanley in :issue:`39207`.)
+Update: This was reverted in 3.9.11, 3.10.3, and 3.11 as the implementation
+could lead to deadlocks. See :issue:`46464`.
 
 curses
 ------
diff --git a/Lib/concurrent/futures/process.py b/Lib/concurrent/futures/process.py
@@ -324,12 +324,6 @@ def run(self):
                 # while waiting on new results.
                 del result_item
 
-                # attempt to increment idle process count
-                executor = self.executor_reference()
-                if executor is not None:
-                    executor._idle_worker_semaphore.release()
-                del executor
-
             if self.is_shutting_down():
                 self.flag_executor_shutting_down()
 
@@ -625,7 +619,6 @@ def __init__(self, max_workers=None, mp_context=None,
         # Shutdown is a two-step process.
         self._shutdown_thread = False
         self._shutdown_lock = threading.Lock()
-        self._idle_worker_semaphore = threading.Semaphore(0)
         self._broken = False
         self._queue_count = 0
         self._pending_work_items = {}
@@ -661,18 +654,21 @@ def __init__(self, max_workers=None, mp_context=None,
     def _start_executor_manager_thread(self):
         if self._executor_manager_thread is None:
             # Start the processes so that their sentinels are known.
+            self._adjust_process_count()
             self._executor_manager_thread = _ExecutorManagerThread(self)
             self._executor_manager_thread.start()
             _threads_wakeups[self._executor_manager_thread] = \
                 self._executor_manager_thread_wakeup
 
     def _adjust_process_count(self):
-        # if there's an idle process, we don't need to spawn a new one.
-        if self._idle_worker_semaphore.acquire(blocking=False):
-            return
-
-        process_count = len(self._processes)
-        if process_count < self._max_workers:
+        # To get rid of this condition don't fork() from this process.
+        # This applies to _any_ thread existing in the process at all, but
+        # that is a long standing issue. We at least make sure this library
+        # is not the cause of its own deadlocks.
+        assert not self._executor_manager_thread, (
+                'Processes cannot be fork()ed after the thread has started, '
+                'deadlock in the child processes could result; bpo-46464.')
+        for _ in range(len(self._processes), self._max_workers):
             p = self._mp_context.Process(
                 target=_process_worker,
                 args=(self._call_queue,
@@ -701,7 +697,6 @@ def submit(self, fn, /, *args, **kwargs):
             # Wake up queue management thread
             self._executor_manager_thread_wakeup.wakeup()
 
-            self._adjust_process_count()
             self._start_executor_manager_thread()
             return f
     submit.__doc__ = _base.Executor.submit.__doc__
diff --git a/Misc/NEWS.d/next/Library/2022-01-24-07-53-50.bpo-46464.Duuez0.rst b/Misc/NEWS.d/next/Library/2022-01-24-07-53-50.bpo-46464.Duuez0.rst
@@ -1,3 +1,7 @@
-Remove the 3.11.0a3 Add ``max_tasks_per_child`` from
-:class:`concurrent.futures.ProcessPoolExecutor` as the implementation relied
-on mixing threads+fork which can cause deadlocks .
+Remove the 3.11.0a3 :issue:`44733` added ``max_tasks_per_child`` feature from
+:class:`concurrent.futures.ProcessPoolExecutor` as the implementation relied on
+mixing threading and fork() which can cause deadlocks in child processes.
+
+Remove the 3.9 :issue:`39207` performance enhancement from that spawned worker
+processes for :class:`concurrent.futures.ProcessPoolExecutor` on demand rather
+than up front as the implementation could cause deadlocks in the child process.
