Skip to content

Commit 88c4836

Browse files
committed
Fix weak container cleanup leak
1 parent c03159e commit 88c4836

File tree

2 files changed

+56
-9
lines changed

2 files changed

+56
-9
lines changed

src/core/di/weak_container.py

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -71,12 +71,19 @@ def register_instance(
7171
if cleanup_callback:
7272
self._cleanup_callbacks[service_type] = cleanup_callback
7373

74-
# Set up weak reference callback for cleanup
75-
def on_delete(ref):
74+
instance_ref = weakref.ref(instance)
75+
76+
# Set up weak reference callback for cleanup without strong reference cycles
77+
def on_delete(_: weakref.ReferenceType[Any]) -> None:
78+
service_instance = instance_ref()
79+
if service_instance is None:
80+
return
7681
try:
77-
cleanup_callback(instance)
78-
except Exception as e:
79-
logger.warning(f"Error in cleanup callback for {service_type}: {e}")
82+
cleanup_callback(service_instance)
83+
except Exception as exc:
84+
logger.warning(
85+
f"Error in cleanup callback for {service_type}: {exc}",
86+
)
8087

8188
weakref.ref(instance, on_delete)
8289

@@ -123,13 +130,17 @@ async def get_service(self, service_type: type[T]) -> T:
123130
# Set up cleanup callback if registered
124131
cleanup_callback = self._cleanup_callbacks.get(service_type)
125132
if cleanup_callback:
133+
instance_ref = weakref.ref(instance)
126134

127-
def on_delete(ref):
135+
def on_delete(_: weakref.ReferenceType[Any]) -> None:
136+
service_instance = instance_ref()
137+
if service_instance is None:
138+
return
128139
try:
129-
cleanup_callback(instance)
130-
except Exception as e:
140+
cleanup_callback(service_instance)
141+
except Exception as exc:
131142
logger.warning(
132-
f"Error in cleanup callback for {service_type}: {e}"
143+
f"Error in cleanup callback for {service_type}: {exc}",
133144
)
134145

135146
weakref.ref(instance, on_delete)
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
"""Tests for the weak DI container memory management."""
2+
3+
from __future__ import annotations
4+
5+
import gc
6+
import weakref
7+
8+
import pytest
9+
from src.core.di.weak_container import WeakDIContainer
10+
11+
12+
class _DummyService:
13+
"""Simple service used to verify garbage collection behavior."""
14+
15+
16+
@pytest.mark.asyncio
17+
async def test_weak_container_allows_garbage_collection() -> None:
18+
"""Instances registered in the weak container should not be leaked."""
19+
20+
container = WeakDIContainer()
21+
22+
def factory() -> _DummyService:
23+
return _DummyService()
24+
25+
container.register_factory(_DummyService, factory)
26+
27+
service = await container.get_service(_DummyService)
28+
service_ref = weakref.ref(service)
29+
30+
# Drop the strong reference held by the test
31+
del service
32+
33+
# Force garbage collection to trigger weakref callbacks
34+
gc.collect()
35+
36+
assert service_ref() is None

0 commit comments

Comments
 (0)