Move finalizers management to SetupState

Author: Anton3

src/_pytest/fixtures.py

@@ -69,10 +69,6 @@
 from _pytest.warning_types import PytestWarning
 
 
-if sys.version_info < (3, 11):
-    from exceptiongroup import BaseExceptionGroup
-
-
 if TYPE_CHECKING:
     from _pytest.python import CallSpec2
     from _pytest.python import Function
@@ -1035,7 +1031,6 @@ def __init__(
         # If the fixture was executed, the current value of the fixture.
         # Can change if the fixture is executed with different parameters.
         self.cached_result: _FixtureCachedResult[FixtureValue] | None = None
-        self._finalizers: Final[nodes.FinalizerStorage] = {}
         # The request object with which the fixture was set up.
         self._cached_request: SubRequest | None = None
         # Handles to remove our finalizer from various scopes.
@@ -1050,39 +1045,20 @@ def scope(self) -> _ScopeName:
         return self._scope.value
 
     def addfinalizer(self, finalizer: Callable[[], object]) -> Callable[[], None]:
-        return nodes.append_finalizer(self._finalizers, finalizer)
+        assert self._cached_request is not None
+        setupstate = self._cached_request.session._setupstate
+        return setupstate.fixture_addfinalizer(finalizer, self)
 
     def finish(self, request: SubRequest) -> None:
-        exceptions: list[BaseException] = []
-        while self._finalizers:
-            _, fin = self._finalizers.popitem()
-            try:
-                fin()
-            except TEST_OUTCOME as e:
-                exceptions.append(e)
-        node = request.node
         try:
-            node.ihook.pytest_fixture_post_finalizer(fixturedef=self, request=request)
-        except TEST_OUTCOME as e:
-            exceptions.append(e)
-
-        # Even if finalization fails, we invalidate the cached fixture
-        # value and remove all finalizers because they may be bound methods
-        # which will keep instances alive.
-        self.cached_result = None
-        self._finalizers.clear()
-        request._fixturemanager._on_fixture_finished(self)
-        self._cached_request = None
-        # Avoid accumulating garbage finalizers in nodes and fixturedefs (#4871).
-        for handle in self._self_finalizer_handles:
-            handle()
-        self._self_finalizer_handles.clear()
-
-        if len(exceptions) == 1:
-            raise exceptions[0]
-        elif len(exceptions) > 1:
-            msg = f'errors while tearing down fixture "{self.argname}" of {node}'
-            raise BaseExceptionGroup(msg, exceptions[::-1])
+            request.session._setupstate.fixture_teardown(self, request.node)
+        finally:
+            self.cached_result = None
+            self._cached_request = None
+            # Avoid accumulating garbage finalizers in nodes and fixturedefs (#4871).
+            for handle in self._self_finalizer_handles:
+                handle()
+            self._self_finalizer_handles.clear()
 
     def execute(self, request: SubRequest) -> FixtureValue:
         """Return the value of this fixture, executing it if not cached."""
@@ -1095,6 +1071,11 @@ def execute(self, request: SubRequest) -> FixtureValue:
         for argname in self.argnames:
             request._get_active_fixturedef(argname)
 
+        self._cached_request = request
+        setupstate = request.session._setupstate
+        setupstate.fixture_setup(self)
+        setupstate.fixture_addfinalizer(self._run_post_finalizer, self)
+
         ihook = request.node.ihook
         try:
             # Setup the fixture, run the code in it, and cache the value
@@ -1103,8 +1084,6 @@ def execute(self, request: SubRequest) -> FixtureValue:
                 fixturedef=self, request=request
             )
         finally:
-            self._cached_request = request
-            request._fixturemanager._on_fixture_setup(self)
             # Schedule our finalizer, even if the setup failed.
             fin = functools.partial(self.finish, request)
             self._self_finalizer_handles.append(request.node.addfinalizer(fin))
@@ -1113,6 +1092,12 @@ def execute(self, request: SubRequest) -> FixtureValue:
 
         return result
 
+    def _run_post_finalizer(self) -> None:
+        request = self._cached_request
+        assert request is not None
+        ihook = request.node.ihook
+        ihook.pytest_fixture_post_finalizer(fixturedef=self, request=request)
+
     def _is_cache_hit(self, old_cache_key: object, new_cache_key: object) -> bool:
         try:
             # Attempt to make a normal == check: this might fail for objects
@@ -1649,8 +1634,6 @@ def __init__(self, session: Session) -> None:
         self._nodeid_autousenames: Final[dict[str, list[str]]] = {
             "": self.config.getini("usefixtures"),
         }
-        # Using dict for keeping insertion order.
-        self._active_fixturedefs: Final[dict[FixtureDef[Any], None]] = {}
         session.config.pluginmanager.register(self, "funcmanage")
 
     def getfixtureinfo(
@@ -1998,25 +1981,6 @@ def _matchfactories(
             if fixturedef.baseid in parentnodeids:
                 yield fixturedef
 
-    def _teardown_stale_fixtures(self, nextitem: nodes.Item) -> None:
-        exceptions: list[BaseException] = []
-        for fixturedef in reversed(list(self._active_fixturedefs.keys())):
-            try:
-                fixturedef._finish_if_param_changed(nextitem)
-            except TEST_OUTCOME as e:
-                exceptions.append(e)
-        if len(exceptions) == 1:
-            raise exceptions[0]
-        elif len(exceptions) > 1:
-            msg = f'errors while tearing down fixtures for "{nextitem.nodeid}"'
-            raise BaseExceptionGroup(msg, exceptions[::-1])
-
-    def _on_fixture_setup(self, fixturedef: FixtureDef[Any]) -> None:
-        self._active_fixturedefs[fixturedef] = None
-
-    def _on_fixture_finished(self, fixturedef: FixtureDef[Any]) -> None:
-        del self._active_fixturedefs[fixturedef]
-
 
 def show_fixtures_per_test(config: Config) -> int | ExitCode:
     from _pytest.main import wrap_session

src/_pytest/nodes.py

@@ -16,7 +16,6 @@
 from typing import NoReturn
 from typing import overload
 from typing import TYPE_CHECKING
-from typing import TypeAlias
 from typing import TypeVar
 import warnings
 
@@ -93,26 +92,6 @@ def _imply_path(
         return Path(fspath)
 
 
-class _FinalizerId:
-    __slots__ = ()
-
-
-Finalizer: TypeAlias = Callable[[], object]
-FinalizerStorage: TypeAlias = dict[_FinalizerId, Finalizer]
-
-
-def append_finalizer(
-    finalizer_storage: FinalizerStorage, finalizer: Finalizer
-) -> Callable[[], None]:
-    finalizer_id = _FinalizerId()
-    finalizer_storage[finalizer_id] = finalizer
-
-    def remove_finalizer() -> None:
-        finalizer_storage.pop(finalizer_id, None)
-
-    return remove_finalizer
-
-
 _NodeType = TypeVar("_NodeType", bound="Node")
 
 

src/_pytest/runner.py

@@ -9,11 +9,13 @@
 import os
 import sys
 import types
+from typing import Any
 from typing import cast
 from typing import final
 from typing import Generic
 from typing import Literal
 from typing import TYPE_CHECKING
+from typing import TypeAlias
 from typing import TypeVar
 
 from .config import Config
@@ -27,10 +29,8 @@
 from _pytest._code.code import TerminalRepr
 from _pytest.config.argparsing import Parser
 from _pytest.deprecated import check_ispytest
-from _pytest.nodes import append_finalizer
 from _pytest.nodes import Collector
 from _pytest.nodes import Directory
-from _pytest.nodes import FinalizerStorage
 from _pytest.nodes import Item
 from _pytest.nodes import Node
 from _pytest.outcomes import Exit
@@ -43,6 +43,7 @@
     from exceptiongroup import BaseExceptionGroup
 
 if TYPE_CHECKING:
+    from _pytest.fixtures import FixtureDef
     from _pytest.main import Session
     from _pytest.terminal import TerminalReporter
 
@@ -433,6 +434,26 @@ def collect() -> list[Item | Collector]:
     return rep
 
 
+class _FinalizerId:
+    __slots__ = ()
+
+
+_Finalizer: TypeAlias = Callable[[], object]
+_FinalizerStorage: TypeAlias = dict[_FinalizerId, _Finalizer]
+
+
+def _append_finalizer(
+    finalizer_storage: _FinalizerStorage, finalizer: _Finalizer
+) -> Callable[[], None]:
+    finalizer_id = _FinalizerId()
+    finalizer_storage[finalizer_id] = finalizer
+
+    def remove_finalizer() -> None:
+        finalizer_storage.pop(finalizer_id, None)
+
+    return remove_finalizer
+
+
 class SetupState:
     """Shared state for setting up/tearing down test items or collectors
     in a session.
@@ -503,11 +524,12 @@ def __init__(self) -> None:
             Node,
             tuple[
                 # Node's finalizers.
-                FinalizerStorage,
+                _FinalizerStorage,
                 # Node's exception and original traceback, if its setup raised.
                 tuple[OutcomeException | Exception, types.TracebackType | None] | None,
             ],
         ] = {}
+        self._fixture_finalizers: dict[FixtureDef[Any], _FinalizerStorage] = {}
 
     def setup(self, item: Item) -> None:
         """Setup objects along the collector chain to the item."""
@@ -523,8 +545,8 @@ def setup(self, item: Item) -> None:
         for col in needed_collectors[len(self.stack) :]:
             assert col not in self.stack
             # Push onto the stack.
-            finalizers = FinalizerStorage()
-            append_finalizer(finalizers, col.teardown)
+            finalizers = _FinalizerStorage()
+            _append_finalizer(finalizers, col.teardown)
             self.stack[col] = (finalizers, None)
             try:
                 col.setup()
@@ -544,7 +566,7 @@ def addfinalizer(
         assert node and not isinstance(node, tuple)
         assert callable(finalizer)
         assert node in self.stack, (node, self.stack)
-        return append_finalizer(self.stack[node][0], finalizer)
+        return _append_finalizer(self.stack[node][0], finalizer)
 
     def teardown_exact(self, nextitem: Item | None) -> None:
         """Teardown the current stack up until reaching nodes that nextitem
@@ -569,7 +591,7 @@ def teardown_exact(self, nextitem: Item | None) -> None:
 
             if isinstance(node, Item) and nextitem is not None:
                 try:
-                    self._teardown_item_towards_nextitem(nextitem)
+                    self._finish_stale_fixtures(nextitem)
                 except TEST_OUTCOME as e:
                     exceptions.append(e)
 
@@ -586,8 +608,48 @@ def teardown_exact(self, nextitem: Item | None) -> None:
         if nextitem is None:
             assert not self.stack
 
-    def _teardown_item_towards_nextitem(self, nextitem: Item) -> None:
-        nextitem.session._fixturemanager._teardown_stale_fixtures(nextitem)
+    def fixture_setup(self, fixturedef: FixtureDef[Any]) -> None:
+        assert fixturedef not in self._fixture_finalizers
+        self._fixture_finalizers[fixturedef] = _FinalizerStorage()
+
+    def fixture_addfinalizer(
+        self, finalizer: Callable[[], object], fixturedef: FixtureDef[Any]
+    ) -> Callable[[], None]:
+        assert fixturedef in self._fixture_finalizers
+        return _append_finalizer(self._fixture_finalizers[fixturedef], finalizer)
+
+    def fixture_teardown(self, fixturedef: FixtureDef[Any], node: Node) -> None:
+        assert fixturedef in self._fixture_finalizers
+        # Do not remove for now to allow adding finalizers last second.
+        finalizers = self._fixture_finalizers[fixturedef]
+
+        exceptions: list[BaseException] = []
+        while finalizers:
+            _, fin = finalizers.popitem()
+            try:
+                fin()
+            except TEST_OUTCOME as e:
+                exceptions.append(e)
+
+        self._fixture_finalizers.pop(fixturedef)
+        if len(exceptions) == 1:
+            raise exceptions[0]
+        elif len(exceptions) > 1:
+            msg = f'errors while tearing down fixture "{fixturedef.argname}" of {node}'
+            raise BaseExceptionGroup(msg, exceptions[::-1])
+
+    def _finish_stale_fixtures(self, nextitem: Item) -> None:
+        exceptions: list[BaseException] = []
+        for fixturedef in reversed(list(self._fixture_finalizers.keys())):
+            try:
+                fixturedef._finish_if_param_changed(nextitem)
+            except TEST_OUTCOME as e:
+                exceptions.append(e)
+        if len(exceptions) == 1:
+            raise exceptions[0]
+        elif len(exceptions) > 1:
+            msg = f'errors while tearing down fixtures for "{nextitem.nodeid}"'
+            raise BaseExceptionGroup(msg, exceptions[::-1])
 
 
 def collect_one_node(collector: Collector) -> CollectReport: