432 resource shutdown order (#473)

* Add PoC

* Add tests for init and shutdown ordering

* Add circular dependencies breaker tests

* Refactoring and sync + async test

* Update changelog

docs/main/changelog.rst

@@ -7,6 +7,12 @@ that were made in every particular version.
 From version 0.7.6 *Dependency Injector* framework strictly 
 follows `Semantic versioning`_
 
+Dev version
+-----------
+- Update ``container.shutdown_resources()`` to respect dependencies order while shutdown.
+  See issue `#432 <https://github.com/ets-labs/python-dependency-injector/issues/432>`_.
+  Thanks to `Saulius Beinorius <https://github.com/saulbein>`_  for bringing up the issue.
+
 4.34.0
 ------
 - Add option ``envs_required`` for configuration provider ``.from_yaml()`` and ``.from_ini()``

src/dependency_injector/containers.pyx

@@ -290,16 +290,46 @@ class DynamicContainer(Container):
 
     def shutdown_resources(self):
         """Shutdown all container resources."""
-        futures = []
+        def _no_initialized_dependencies(resource):
+            for related in resource.related:
+                if isinstance(related, providers.Resource) and related.initialized:
+                    return False
+            return True
 
-        for provider in self.traverse(types=[providers.Resource]):
-            shutdown = provider.shutdown()
+        def _without_initialized_dependencies(resources):
+            return list(filter(_no_initialized_dependencies, resources))
 
-            if __is_future_or_coroutine(shutdown):
-                futures.append(shutdown)
+        def _any_initialized(resources):
+            return any(resource.initialized for resource in resources)
 
-        if futures:
-            return asyncio.gather(*futures)
+        def _any_in_async_mode(resources):
+            return any(resource.is_async_mode_enabled() for resource in resources)
+
+        async def _async_ordered_shutdown(resources):
+            while _any_initialized(resources):
+                resources_to_shutdown = _without_initialized_dependencies(resources)
+                if not resources_to_shutdown:
+                    raise RuntimeError('Unable to resolve resources shutdown order')
+                futures = []
+                for resource in resources_to_shutdown:
+                    result = resource.shutdown()
+                    if __is_future_or_coroutine(result):
+                        futures.append(result)
+                await asyncio.gather(*futures)
+
+        def _sync_ordered_shutdown(resources):
+            while _any_initialized(resources):
+                resources_to_shutdown = _without_initialized_dependencies(resources)
+                if not resources_to_shutdown:
+                    raise RuntimeError('Unable to resolve resources shutdown order')
+                for resource in resources_to_shutdown:
+                    resource.shutdown()
+
+        resources = list(self.traverse(types=[providers.Resource]))
+        if _any_in_async_mode(resources):
+            return _async_ordered_shutdown(resources)
+        else:
+            return _sync_ordered_shutdown(resources)
 
     def apply_container_providers_overridings(self):
         """Apply container providers' overridings."""

tests/unit/containers/test_dynamic_async_resources_py36.py

@@ -1,6 +1,5 @@
 """Dependency injector dynamic container unit tests for async resources."""
-
-import unittest
+import asyncio
 
 # Runtime import to get asyncutils module
 import os
@@ -23,49 +22,138 @@ from dependency_injector import (
 
 class AsyncResourcesTest(AsyncTestCase):
 
-    @unittest.skipIf(sys.version_info[:2] <= (3, 5), 'Async test')
-    def test_async_init_resources(self):
-        async def _init1():
-            _init1.init_counter += 1
-            yield
-            _init1.shutdown_counter += 1
+    def test_init_and_shutdown_ordering(self):
+        """Test init and shutdown resources.
 
-        _init1.init_counter = 0
-        _init1.shutdown_counter = 0
+        Methods .init_resources() and .shutdown_resources() should respect resources dependencies.
+        Initialization should first initialize resources without dependencies and then provide
+        these resources to other resources. Resources shutdown should follow the same rule: first
+        shutdown resources without initialized dependencies and then continue correspondingly
+        until all resources are shutdown.
+        """
+        initialized_resources = []
+        shutdown_resources = []
 
-        async def _init2():
-            _init2.init_counter += 1
-            yield
-            _init2.shutdown_counter += 1
+        async def _resource(name, delay, **_):
+            await asyncio.sleep(delay)
+            initialized_resources.append(name)
 
-        _init2.init_counter = 0
-        _init2.shutdown_counter = 0
+            yield name
+
+            await asyncio.sleep(delay)
+            shutdown_resources.append(name)
 
         class Container(containers.DeclarativeContainer):
-            resource1 = providers.Resource(_init1)
-            resource2 = providers.Resource(_init2)
+            resource1 = providers.Resource(
+                _resource,
+                name='r1',
+                delay=0.03,
+            )
+            resource2 = providers.Resource(
+                _resource,
+                name='r2',
+                delay=0.02,
+                r1=resource1,
+            )
+            resource3 = providers.Resource(
+                _resource,
+                name='r3',
+                delay=0.01,
+                r2=resource2,
+            )
 
         container = Container()
-        self.assertEqual(_init1.init_counter, 0)
-        self.assertEqual(_init1.shutdown_counter, 0)
-        self.assertEqual(_init2.init_counter, 0)
-        self.assertEqual(_init2.shutdown_counter, 0)
 
         self._run(container.init_resources())
-        self.assertEqual(_init1.init_counter, 1)
-        self.assertEqual(_init1.shutdown_counter, 0)
-        self.assertEqual(_init2.init_counter, 1)
-        self.assertEqual(_init2.shutdown_counter, 0)
+        self.assertEqual(initialized_resources, ['r1', 'r2', 'r3'])
+        self.assertEqual(shutdown_resources, [])
 
         self._run(container.shutdown_resources())
-        self.assertEqual(_init1.init_counter, 1)
-        self.assertEqual(_init1.shutdown_counter, 1)
-        self.assertEqual(_init2.init_counter, 1)
-        self.assertEqual(_init2.shutdown_counter, 1)
+        self.assertEqual(initialized_resources, ['r1', 'r2', 'r3'])
+        self.assertEqual(shutdown_resources, ['r1', 'r2', 'r3'])
 
         self._run(container.init_resources())
+        self.assertEqual(initialized_resources, ['r1', 'r2', 'r3', 'r1', 'r2', 'r3'])
+        self.assertEqual(shutdown_resources, ['r1', 'r2', 'r3'])
+
         self._run(container.shutdown_resources())
-        self.assertEqual(_init1.init_counter, 2)
-        self.assertEqual(_init1.shutdown_counter, 2)
-        self.assertEqual(_init2.init_counter, 2)
-        self.assertEqual(_init2.shutdown_counter, 2)
+        self.assertEqual(initialized_resources, ['r1', 'r2', 'r3', 'r1', 'r2', 'r3'])
+        self.assertEqual(shutdown_resources, ['r1', 'r2', 'r3', 'r1', 'r2', 'r3'])
+
+    def test_shutdown_circular_dependencies_breaker(self):
+        async def _resource(name, **_):
+            yield name
+
+        class Container(containers.DeclarativeContainer):
+            resource1 = providers.Resource(
+                _resource,
+                name='r1',
+            )
+            resource2 = providers.Resource(
+                _resource,
+                name='r2',
+                r1=resource1,
+            )
+            resource3 = providers.Resource(
+                _resource,
+                name='r3',
+                r2=resource2,
+            )
+
+        container = Container()
+        self._run(container.init_resources())
+
+        # Create circular dependency after initialization (r3 -> r2 -> r1 -> r3 -> ...)
+        container.resource1.add_kwargs(r3=container.resource3)
+
+        with self.assertRaises(RuntimeError) as context:
+            self._run(container.shutdown_resources())
+        self.assertEqual(str(context.exception), 'Unable to resolve resources shutdown order')
+
+    def test_shutdown_sync_and_async_ordering(self):
+        initialized_resources = []
+        shutdown_resources = []
+
+        def _sync_resource(name, **_):
+            initialized_resources.append(name)
+            yield name
+            shutdown_resources.append(name)
+
+        async def _async_resource(name, **_):
+            initialized_resources.append(name)
+            yield name
+            shutdown_resources.append(name)
+
+        class Container(containers.DeclarativeContainer):
+            resource1 = providers.Resource(
+                _sync_resource,
+                name='r1',
+            )
+            resource2 = providers.Resource(
+                _sync_resource,
+                name='r2',
+                r1=resource1,
+            )
+            resource3 = providers.Resource(
+                _async_resource,
+                name='r3',
+                r2=resource2,
+            )
+
+        container = Container()
+
+        self._run(container.init_resources())
+        self.assertEqual(initialized_resources, ['r1', 'r2', 'r3'])
+        self.assertEqual(shutdown_resources, [])
+
+        self._run(container.shutdown_resources())
+        self.assertEqual(initialized_resources, ['r1', 'r2', 'r3'])
+        self.assertEqual(shutdown_resources, ['r1', 'r2', 'r3'])
+
+        self._run(container.init_resources())
+        self.assertEqual(initialized_resources, ['r1', 'r2', 'r3', 'r1', 'r2', 'r3'])
+        self.assertEqual(shutdown_resources, ['r1', 'r2', 'r3'])
+
+        self._run(container.shutdown_resources())
+        self.assertEqual(initialized_resources, ['r1', 'r2', 'r3', 'r1', 'r2', 'r3'])
+        self.assertEqual(shutdown_resources, ['r1', 'r2', 'r3', 'r1', 'r2', 'r3'])

tests/unit/containers/test_dynamic_py2_py3.py

@@ -192,51 +192,86 @@ class DeclarativeContainerInstanceTests(unittest.TestCase):
         self.assertEqual(container.overridden, tuple())
         self.assertEqual(container.p11.overridden, tuple())
 
-    def test_init_shutdown_resources(self):
-        def _init1():
-            _init1.init_counter += 1
-            yield
-            _init1.shutdown_counter += 1
+    def test_init_and_shutdown_resources_ordering(self):
+        """Test init and shutdown resources.
 
-        _init1.init_counter = 0
-        _init1.shutdown_counter = 0
+        Methods .init_resources() and .shutdown_resources() should respect resources dependencies.
+        Initialization should first initialize resources without dependencies and then provide
+        these resources to other resources. Resources shutdown should follow the same rule: first
+        shutdown resources without initialized dependencies and then continue correspondingly
+        until all resources are shutdown.
+        """
+        initialized_resources = []
+        shutdown_resources = []
 
-        def _init2():
-            _init2.init_counter += 1
-            yield
-            _init2.shutdown_counter += 1
-
-        _init2.init_counter = 0
-        _init2.shutdown_counter = 0
+        def _resource(name, **_):
+            initialized_resources.append(name)
+            yield name
+            shutdown_resources.append(name)
 
         class Container(containers.DeclarativeContainer):
-            resource1 = providers.Resource(_init1)
-            resource2 = providers.Resource(_init2)
+            resource1 = providers.Resource(
+                _resource,
+                name='r1',
+            )
+            resource2 = providers.Resource(
+                _resource,
+                name='r2',
+                r1=resource1,
+            )
+            resource3 = providers.Resource(
+                _resource,
+                name='r3',
+                r2=resource2,
+            )
 
         container = Container()
-        self.assertEqual(_init1.init_counter, 0)
-        self.assertEqual(_init1.shutdown_counter, 0)
-        self.assertEqual(_init2.init_counter, 0)
-        self.assertEqual(_init2.shutdown_counter, 0)
 
         container.init_resources()
-        self.assertEqual(_init1.init_counter, 1)
-        self.assertEqual(_init1.shutdown_counter, 0)
-        self.assertEqual(_init2.init_counter, 1)
-        self.assertEqual(_init2.shutdown_counter, 0)
+        self.assertEqual(initialized_resources, ['r1', 'r2', 'r3'])
+        self.assertEqual(shutdown_resources, [])
 
         container.shutdown_resources()
-        self.assertEqual(_init1.init_counter, 1)
-        self.assertEqual(_init1.shutdown_counter, 1)
-        self.assertEqual(_init2.init_counter, 1)
-        self.assertEqual(_init2.shutdown_counter, 1)
+        self.assertEqual(initialized_resources, ['r1', 'r2', 'r3'])
+        self.assertEqual(shutdown_resources, ['r1', 'r2', 'r3'])
 
         container.init_resources()
+        self.assertEqual(initialized_resources, ['r1', 'r2', 'r3', 'r1', 'r2', 'r3'])
+        self.assertEqual(shutdown_resources, ['r1', 'r2', 'r3'])
+
         container.shutdown_resources()
-        self.assertEqual(_init1.init_counter, 2)
-        self.assertEqual(_init1.shutdown_counter, 2)
-        self.assertEqual(_init2.init_counter, 2)
-        self.assertEqual(_init2.shutdown_counter, 2)
+        self.assertEqual(initialized_resources, ['r1', 'r2', 'r3', 'r1', 'r2', 'r3'])
+        self.assertEqual(shutdown_resources, ['r1', 'r2', 'r3', 'r1', 'r2', 'r3'])
+
+    def test_shutdown_resources_circular_dependencies_breaker(self):
+        def _resource(name, **_):
+            yield name
+
+        class Container(containers.DeclarativeContainer):
+            resource1 = providers.Resource(
+                _resource,
+                name='r1',
+            )
+            resource2 = providers.Resource(
+                _resource,
+                name='r2',
+                r1=resource1,
+            )
+            resource3 = providers.Resource(
+                _resource,
+                name='r3',
+                r2=resource2,
+            )
+
+        container = Container()
+        container.init_resources()
+
+        # Create circular dependency after initialization (r3 -> r2 -> r1 -> r3 -> ...)
+        container.resource1.add_kwargs(r3=container.resource3)
+
+        with self.assertRaises(RuntimeError) as context:
+            container.shutdown_resources()
+        self.assertEqual(str(context.exception), 'Unable to resolve resources shutdown order')
 
     def test_init_shutdown_nested_resources(self):
         def _init1():