Skip to content

fix: old provider not shutting down when the new provider fails to initialize#98

Open
NeaguGeorgiana23 wants to merge 5 commits into
mainfrom
shut_down_old_provider
Open

fix: old provider not shutting down when the new provider fails to initialize#98
NeaguGeorgiana23 wants to merge 5 commits into
mainfrom
shut_down_old_provider

Conversation

@NeaguGeorgiana23
Copy link
Copy Markdown
Contributor

This PR

Ensures that a displaced feature provider is always shut down when a new provider is set, preventing resource leaks when the new provider fails to initialize.
Previously, the old provider was removed from the internal repository map immediately upon setting a new one, but Shutdown() was only called if the new provider successfully initialized. If the new provider's Init() failed, the old provider remained active but unreferenced, leaking its resources.

Related Issues

Fixes #97

…tialize.

Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
@NeaguGeorgiana23 NeaguGeorgiana23 requested review from a team as code owners May 12, 2026 09:23
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request modifies the ProviderRepository to unconditionally shut down the old provider during initialization, regardless of whether the new provider successfully reaches a ready state. Correspondingly, the test suite has been updated to verify that the old provider is indeed shut down even when the new provider's initialization fails. I have no feedback to provide.

Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants