Description
BaseProvider.close() does nothing, but BaseProvider.__init__ creates an httpx.Client:
https://github.com/sugarlabs/sugar-ai/blob/main/app/providers/base.py
def close(self) -> None:
"""Release provider resources."""
pass
RAGAgent.set_model() calls old_provider.close() when swapping providers, expecting the previous one to release its resources:
def set_model(self, provider: BaseProvider) -> None:
old_provider = self.provider
self.provider = provider
self.model_name = provider.get_model_name()
if old_provider is not provider:
try:
old_provider.close()
except Exception as e:
logger.warning("Failed to close previous provider: %s", e)
Because close() is a no-op, every model switch leaks a client along with its connection pool and open sockets. With the model-switching endpoint (#121) this accumulates over the life of the process.
Affected providers
| Provider |
Creates httpx.Client |
Overrides close() |
Leaks |
BaseProvider |
yes |
no |
yes |
GeminiProvider |
yes |
no |
yes |
OllamaProvider |
yes |
yes |
no |
HuggingFaceProvider |
no (local model) |
no |
n/a |
OllamaProvider is unaffected only because it happens to override close() correctly.
Reproduction
from app.providers.base import BaseProvider
p = BaseProvider(model_name="gpt-4o-mini", api_key="test-key")
print(p._client.is_closed) # False
p.close()
print(p._client.is_closed) # False <-- still open, should be True
Expected
close() releases the HTTP client, so p._client.is_closed is True afterwards.
Note on the fix
The fix belongs in BaseProvider so all providers inherit it, rather than in each subclass. It needs a guard rather than a bare self._client.close(): HuggingFaceProvider subclasses BaseProvider but loads a local model and never calls super().__init__(), so it has no _client, and an unguarded close raises AttributeError there.
I have a fix with tests ready in #171.
Description
BaseProvider.close()does nothing, butBaseProvider.__init__creates anhttpx.Client:https://github.com/sugarlabs/sugar-ai/blob/main/app/providers/base.py
RAGAgent.set_model()callsold_provider.close()when swapping providers, expecting the previous one to release its resources:Because
close()is a no-op, every model switch leaks a client along with its connection pool and open sockets. With the model-switching endpoint (#121) this accumulates over the life of the process.Affected providers
httpx.Clientclose()BaseProviderGeminiProviderOllamaProviderHuggingFaceProviderOllamaProvideris unaffected only because it happens to overrideclose()correctly.Reproduction
Expected
close()releases the HTTP client, sop._client.is_closedisTrueafterwards.Note on the fix
The fix belongs in
BaseProviderso all providers inherit it, rather than in each subclass. It needs a guard rather than a bareself._client.close():HuggingFaceProvidersubclassesBaseProviderbut loads a local model and never callssuper().__init__(), so it has no_client, and an unguarded close raisesAttributeErrorthere.I have a fix with tests ready in #171.