Fix python-poetry#2658: Run all uninstalls serially

poetry/installation/executor.py

@@ -103,29 +103,48 @@ def execute(self, operations):  # type: (Operation) -> int
             self._display_summary(operations)
 
         # We group operations by priority
-        groups = itertools.groupby(operations, key=lambda o: -o.priority)
+        unique = 0
+
+        def groupkey(op):
+            if op.job_type == "uninstall":
+                # See Github issue https://github.com/python-poetry/poetry/issues/2658
+                # Running uninstalls in parallel is not safe. Therefore, we run each
+                # uninstall in its separate group.
+                nonlocal unique
+                unique += 1
+                return unique
+            else:
+                # We group the rest of the operations by priority.
+                return -op.priority
+
+        groups = itertools.groupby(operations, key=groupkey)
         self._sections = OrderedDict()
         for _, group in groups:
-            tasks = []
-            for operation in group:
-                if self._shutdown:
-                    break
-
-                tasks.append(self._executor.submit(self._execute_operation, operation))
+            if self._execute_group(tuple(group)) == 1:
+                return 1
 
-            try:
-                wait(tasks)
-            except KeyboardInterrupt:
-                self._shutdown = True
+        return 0
 
+    def _execute_group(self, group):  # type: (Operation) -> int
+        tasks = []
+        for operation in group:
             if self._shutdown:
-                # Cancelling further tasks from being executed
-                [task.cancel() for task in tasks]
-                self._executor.shutdown(wait=True)
-
                 break
 
-        return 1 if self._shutdown else 0
+            tasks.append(self._executor.submit(self._execute_operation, operation))
+
+        try:
+            wait(tasks)
+        except KeyboardInterrupt:
+            self._shutdown = True
+
+        if self._shutdown:
+            # Cancelling further tasks from being executed
+            [task.cancel() for task in tasks]
+            self._executor.shutdown(wait=True)
+            return 1
+
+        return 0
 
     def _write(self, operation, line):
         if not self.supports_fancy_output() or not self._should_write_operation(

tests/installation/test_executor.py

@@ -4,6 +4,8 @@
 import re
 import shutil
 
+from unittest import mock
+
 import pytest
 
 from clikit.api.formatter.style import Style
@@ -126,6 +128,29 @@ def test_execute_executes_a_batch_of_operations(
     assert 5 == len(env.executed)
 
 
+def test_execute_uninstalls_serially(config, io):
+    executor = Executor(None, None, Config(), io)
+    executor._execute_group = mock.Mock()
+
+    assert 0 == executor.execute(
+        [
+            Uninstall(Package("pytest", "3.5.2")),
+            Uninstall(Package("attrs", "17.4.0")),
+            Uninstall(Package("requests", "2.18.3")),
+            Uninstall(Package("clikit", "0.2.3")),
+        ]
+    )
+
+    executed_groups = []
+    for call in executor._execute_group.mock_calls:
+        ops = call.args[0]
+        assert all(op.job_type == "uninstall" for op in ops)
+        executed_groups.append({op.package.name for op in ops})
+
+    # Uninstalls executed serially in separate groups.
+    assert executed_groups == [{"pytest"}, {"attrs"}, {"requests"}, {"clikit"}]
+
+
 def test_execute_shows_skipped_operations_if_verbose(config, pool, io):
     config = Config()
     config.merge({"cache-dir": "/foo"})