hooks: fix plugins registering other plugins in a hook

src/pluggy/_hooks.py

@@ -497,7 +497,8 @@ def __call__(self, **kwargs: object) -> Any:
         ), "Cannot directly call a historic hook - use call_historic instead."
         self._verify_all_args_are_provided(kwargs)
         firstresult = self.spec.opts.get("firstresult", False) if self.spec else False
-        return self._hookexec(self.name, self._hookimpls, kwargs, firstresult)
+        # Copy because plugins may register other plugins during iteration (#438).
+        return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
 
     def call_historic(
         self,
@@ -518,7 +519,8 @@ def call_historic(
         self._call_history.append((kwargs, result_callback))
         # Historizing hooks don't return results.
         # Remember firstresult isn't compatible with historic.
-        res = self._hookexec(self.name, self._hookimpls, kwargs, False)
+        # Copy because plugins may register other plugins during iteration (#438).
+        res = self._hookexec(self.name, self._hookimpls.copy(), kwargs, False)
         if result_callback is None:
             return
         if isinstance(res, list):

testing/test_pluginmanager.py

@@ -675,3 +675,79 @@ def he_method1(self):
         assert saveindent[0] > indent
     finally:
         undo()
+
+
+@pytest.mark.parametrize("historic", [False, True])
+def test_register_while_calling(
+    pm: PluginManager,
+    historic: bool,
+) -> None:
+    """Test that registering an impl of a hook while it is being called does
+    not affect the call itself, only later calls.
+
+    For historic hooks however, the hook is called immediately on registration.
+
+    Regression test for #438.
+    """
+    hookspec = HookspecMarker("example")
+
+    class Hooks:
+        @hookspec(historic=historic)
+        def configure(self) -> int:
+            raise NotImplementedError()
+
+    class Plugin1:
+        @hookimpl
+        def configure(self) -> int:
+            return 1
+
+    class Plugin2:
+        def __init__(self) -> None:
+            self.already_registered = False
+
+        @hookimpl
+        def configure(self) -> int:
+            if not self.already_registered:
+                pm.register(Plugin4())
+                pm.register(Plugin5())
+                pm.register(Plugin6())
+                self.already_registered = True
+            return 2
+
+    class Plugin3:
+        @hookimpl
+        def configure(self) -> int:
+            return 3
+
+    class Plugin4:
+        @hookimpl(tryfirst=True)
+        def configure(self) -> int:
+            return 4
+
+    class Plugin5:
+        @hookimpl()
+        def configure(self) -> int:
+            return 5
+
+    class Plugin6:
+        @hookimpl(trylast=True)
+        def configure(self) -> int:
+            return 6
+
+    pm.add_hookspecs(Hooks)
+    pm.register(Plugin1())
+    pm.register(Plugin2())
+    pm.register(Plugin3())
+
+    if not historic:
+        result = pm.hook.configure()
+        assert result == [3, 2, 1]
+        result = pm.hook.configure()
+        assert result == [4, 5, 3, 2, 1, 6]
+    else:
+        result = []
+        pm.hook.configure.call_historic(result.append)
+        assert result == [4, 5, 6, 3, 2, 1]
+        result = []
+        pm.hook.configure.call_historic(result.append)
+        assert result == [4, 5, 3, 2, 1, 6]