Don't reregister subfixture finalizer in parent fixture if value is cached.

changelog/12135.bugfix.rst

@@ -0,0 +1 @@
+Fix subfixtures adding their finalizer multiple times to parent fixtures, causing incorrect teardown ordering in some instances.

src/_pytest/fixtures.py

@@ -1035,13 +1035,18 @@ class FixtureDef(Generic[FixtureValue]):
             raise BaseExceptionGroup(msg, exceptions[::-1])
 
     def execute(self, request: SubRequest) -> FixtureValue:
-        finalizer = functools.partial(self.finish, request=request)
-        # Get required arguments and register our own finish()
-        # with their finalization.
+        # Ensure arguments (parent fixtures) are loaded.
+        # If their cache has been invalidated it will finish itself and subfixtures,
+        # which will set our self.cached_result = None.
+        # If/when parent fixture parametrization is included in our cache key this
+        # can be moved after checking our cache key and not require saving in a list.
+        parent_fixtures_to_add_finalizer = []
         for argname in self.argnames:
             fixturedef = request._get_active_fixturedef(argname)
             if not isinstance(fixturedef, PseudoFixtureDef):
-                fixturedef.addfinalizer(finalizer)
+                # save fixture as one to add our finalizer to, if we're not cached
+                # resolves #12135
+                parent_fixtures_to_add_finalizer.append(fixturedef)
 
         my_cache_key = self.cache_key(request)
         if self.cached_result is not None:
@@ -1060,6 +1065,11 @@ class FixtureDef(Generic[FixtureValue]):
             self.finish(request)
             assert self.cached_result is None
 
+        finalizer = functools.partial(self.finish, request=request)
+        # add finalizer to parent fixtures
+        for parent_fixture in parent_fixtures_to_add_finalizer:
+            parent_fixture.addfinalizer(finalizer)
+
         ihook = request.node.ihook
         try:
             # Setup the fixture, run the code in it, and cache the value

testing/python/fixtures.py

@@ -4751,3 +4751,88 @@ def test_scoped_fixture_teardown_order(pytester: Pytester) -> None:
     )
     result = pytester.runpytest()
     assert result.ret == 0
+
+
+def test_subfixture_not_queue_multiple_finalizers(pytester: Pytester) -> None:
+    """Make sure subfixtures don't needlessly requeue their finalizer multiple times in
+    parent fixture.
+
+    Regression test for #12135
+    """
+    pytester.makepyfile(
+        """
+        import pytest
+
+        def _assert_len(request):
+            assert len(request._get_active_fixturedef('fixture_1')._finalizers) == 1
+
+
+        @pytest.fixture(scope="module")
+        def fixture_1():
+            ...
+
+        @pytest.fixture(scope="module")
+        def fixture_2(request, fixture_1):
+            yield
+
+        def test_1(request, fixture_2):
+            _assert_len(request)
+
+        def test_2(request, fixture_2):
+            _assert_len(request)
+        """
+    )
+    result = pytester.runpytest()
+    assert result.ret == 0
+
+
+def test_subfixture_teardown_order(pytester: Pytester) -> None:
+    """
+    Make sure fixtures don't re-register their finalization in parent fixtures multiple
+    times, causing ordering failure in their teardowns.
+
+    Regression test for #12135
+    """
+    pytester.makepyfile(
+        """
+        import pytest
+
+        execution_order = []
+
+        @pytest.fixture(scope="class")
+        def fixture_1():
+            ...
+
+        @pytest.fixture(scope="class")
+        def fixture_2(fixture_1):
+            execution_order.append("setup 2")
+            yield
+            execution_order.append("teardown 2")
+
+        @pytest.fixture(scope="class")
+        def fixture_3(fixture_1):
+            execution_order.append("setup 3")
+            yield
+            execution_order.append("teardown 3")
+
+        class TestFoo:
+            def test_initialize_fixtures(self, fixture_2, fixture_3):
+                ...
+
+            # This would previously reschedule fixture_2's finalizer in the parent fixture,
+            # causing it to be torn down before fixture 3.
+            def test_reschedule_fixture_2(self, fixture_2):
+                ...
+
+            # Force finalization directly on fixture_1
+            # Otherwise the cleanup would sequence 3&2 before 1 as normal.
+            @pytest.mark.parametrize("fixture_1", [None], indirect=["fixture_1"])
+            def test_finalize_fixture_1(self, fixture_1):
+                ...
+
+        def test_result():
+            assert execution_order == ["setup 2", "setup 3", "teardown 3", "teardown 2"]
+        """
+    )
+    result = pytester.runpytest()
+    assert result.ret == 0