fix: finalize a build that finishes during a close in that close

docs/providers/lifecycle.md

@@ -246,10 +246,10 @@ The container counts as closed as soon as `close_sync()` or `close_async()` star
 that resolves from its own container gets `ContainerClosedError`. Give a finalizer what it needs
 through the cached instance.
 
-Closing does not wait for resolves running on other threads. If another thread is still building a
-cached instance when the close starts, that close does not finalize it: the instance stays cached in
-the closed container, and its finalizer runs at the next close. Stop resolving from a container
-before you close it.
+Closing does not wait for resolves running on other threads. If another thread finishes building a
+cached instance while a close runs, that close finalizes it. If it finishes after the close returns,
+the instance stays cached in the closed container, and its finalizer runs at the next close. Stop
+resolving from a container before you close it.
 
 After close, resolving raises `ContainerClosedError` and the creator does not run. That includes a
 resolve through a child that reaches the closed container. A child of a closed container can still

modern_di/cache.py

@@ -47,10 +47,10 @@ def cached_count(items: dict[int, CacheItem]) -> int:
 
 
 async def close_async(creation_order: list[CacheItem]) -> None:
-    """Close the items queued when this starts, newest first, and dequeue them; failures raise together."""
+    """Pop and close items newest first until ``creation_order`` is empty; failures raise together at the end."""
     finalizer_errors: list[Exception] = []
-    closing = creation_order[:]
-    for cache_item in reversed(closing):
+    while creation_order:
+        cache_item = creation_order.pop()
         finalizer = cache_item.settings.finalizer
         if finalizer is not None and cache_item.cache is not types.UNSET and not cache_item.finalized:
             try:
@@ -60,20 +60,22 @@ async def close_async(creation_order: list[CacheItem]) -> None:
             except Exception as e:  # noqa: BLE001
                 e.add_note(finalizer_note(type(cache_item.cache)))
                 finalizer_errors.append(e)
+            except BaseException:
+                creation_order.append(cache_item)
+                raise
             else:
                 cache_item.finalized = True
         cache_item.clear()
-    del creation_order[: len(closing)]
     if finalizer_errors:
         raise exceptions.FinalizerError(exceptions=finalizer_errors, is_async=True)
 
 
 def close_sync(creation_order: list[CacheItem]) -> None:
-    """Close the items queued when this starts, newest first; an async finalizer stays queued for an async close."""
+    """Pop and close items newest first until ``creation_order`` is empty; an async finalizer is queued again."""
     finalizer_errors: list[Exception] = []
     remaining: list[CacheItem] = []
-    closing = creation_order[:]
-    for cache_item in reversed(closing):
+    while creation_order:
+        cache_item = creation_order.pop()
         finalizer = cache_item.settings.finalizer
         value = cache_item.cache
         if finalizer is not None and value is not types.UNSET and not cache_item.finalized:
@@ -86,6 +88,9 @@ def close_sync(creation_order: list[CacheItem]) -> None:
             except Exception as e:  # noqa: BLE001
                 e.add_note(finalizer_note(type(value)))
                 finalizer_errors.append(e)
+            except BaseException:
+                creation_order.extend([cache_item, *reversed(remaining)])
+                raise
             else:
                 if result is not None and inspect.isawaitable(result):
                     if inspect.iscoroutine(result):
@@ -96,6 +101,6 @@ def close_sync(creation_order: list[CacheItem]) -> None:
                 cache_item.finalized = True
         cache_item.clear()
     remaining.reverse()
-    creation_order[: len(closing)] = remaining
+    creation_order[:0] = remaining
     if finalizer_errors:
         raise exceptions.FinalizerError(exceptions=finalizer_errors, is_async=False)

modern_di/container.py

@@ -262,8 +262,8 @@ async def close_async(self) -> None:
         Overrides are kept. A resolve from inside a finalizer raises
         :class:`~modern_di.exceptions.ContainerClosedError`. Every finalizer runs even when one
         raises; the failures come back together as one :class:`~modern_di.exceptions.FinalizerError`.
-        Not coordinated with resolves on other threads: an instance still being built is finalized by
-        the next close.
+        Not coordinated with resolves on other threads: an instance that finishes building while this
+        runs is finalized by it, and one that finishes later by the next close.
         """
         self._closed = True
         if self._creation_order:
@@ -276,7 +276,8 @@ def close_sync(self) -> None:
         :class:`~modern_di.exceptions.ContainerClosedError`. Every finalizer runs even when one
         raises; the failures come back together as one :class:`~modern_di.exceptions.FinalizerError`.
         An async finalizer fails here and stays pending for a later :meth:`close_async`. Not coordinated
-        with resolves on other threads: an instance still being built is finalized by the next close.
+        with resolves on other threads: an instance that finishes building while this runs is finalized
+        by it, and one that finishes later by the next close.
         """
         self._closed = True
         if self._creation_order:

tests/providers/test_cached_factory.py

@@ -681,6 +681,9 @@ class _First: ...
 class _Second: ...
 
 
+class _Third: ...
+
+
 class _Built:
     count = 0
 
@@ -823,6 +826,40 @@ class SlowGroup(Group):
     assert container.resolve(_Second) is not second
 
 
+async def test_interrupted_close_sync_keeps_unfinalized_items_queued() -> None:
+    events: list[str] = []
+
+    def sync_finalizer(_: _First) -> None:
+        events.append("first")
+
+    def interrupting_finalizer(_: _Second) -> None:
+        events.append("second")
+        if events.count("second") == 1:
+            raise KeyboardInterrupt
+
+    async def async_finalizer(_: _Third) -> None:
+        events.append("third")
+
+    class InterruptGroup(Group):
+        first = providers.Factory(creator=_First, cache=providers.CacheSettings(finalizer=sync_finalizer))
+        second = providers.Factory(creator=_Second, cache=providers.CacheSettings(finalizer=interrupting_finalizer))
+        third = providers.Factory(creator=_Third, cache=providers.CacheSettings(finalizer=async_finalizer))
+
+    container = Container(groups=[InterruptGroup])
+    container.resolve(_First)
+    container.resolve(_Second)
+    container.resolve(_Third)
+
+    with pytest.raises(KeyboardInterrupt):
+        container.close_sync()
+
+    assert events == ["second"]
+
+    await container.close_async()
+
+    assert events == ["second", "third", "second", "first"]
+
+
 async def test_close_async_after_a_cancelled_close_does_not_refinalize_closed_items() -> None:
     events: list[str] = []
 

tests/test_cache.py

@@ -203,12 +203,12 @@ class G(Group):
     return container, log
 
 
-def test_a_build_finishing_during_close_sync_is_finalized_by_the_next_close() -> None:
+def test_a_build_finishing_during_close_sync_is_finalized_by_that_close() -> None:
     log: list[str] = []
     container, closed = _close_with_a_build_finishing_mid_close(
         Container.close_sync, x_finalizer=lambda _: log.append("X")
     )
-    assert (closed, log) == (["Y"], [])
+    assert (closed, log) == (["Y"], ["X"])
 
     container.close_sync()
 
@@ -219,10 +219,10 @@ def _close_async(container: Container) -> None:
     asyncio.run(container.close_async())
 
 
-def test_a_build_finishing_during_close_async_is_finalized_by_the_next_close() -> None:
+def test_a_build_finishing_during_close_async_is_finalized_by_that_close() -> None:
     log: list[str] = []
     container, closed = _close_with_a_build_finishing_mid_close(_close_async, x_finalizer=lambda _: log.append("X"))
-    assert (closed, log) == (["Y"], [])
+    assert (closed, log) == (["Y"], ["X"])
 
     _close_async(container)
 
@@ -235,9 +235,13 @@ def test_a_build_finishing_during_close_sync_keeps_an_async_finalizer_pending()
     async def finalize_x(_: object) -> None:
         log.append("X")
 
-    container, _ = _close_with_a_build_finishing_mid_close(Container.close_sync, x_finalizer=finalize_x)
-    with pytest.raises(FinalizerError):
-        container.close_sync()
+    def close_sync_reporting_the_async_finalizer(container: Container) -> None:
+        with pytest.raises(FinalizerError):
+            container.close_sync()
+
+    container, _ = _close_with_a_build_finishing_mid_close(
+        close_sync_reporting_the_async_finalizer, x_finalizer=finalize_x
+    )
     assert log == []
 
     _close_async(container)