Skip to content

Close receiver when stopping LatestValueCache#495

Open
shsms wants to merge 4 commits intofrequenz-floss:v1.x.xfrom
shsms:improve-latest-value-cache-cleanup
Open

Close receiver when stopping LatestValueCache#495
shsms wants to merge 4 commits intofrequenz-floss:v1.x.xfrom
shsms:improve-latest-value-cache-cleanup

Conversation

@shsms
Copy link
Contributor

@shsms shsms commented Feb 16, 2026

No description provided.

Copilot AI review requested due to automatic review settings February 16, 2026 14:05
@shsms shsms requested a review from a team as a code owner February 16, 2026 14:05
@shsms shsms requested review from simonvoelcker and removed request for a team February 16, 2026 14:05
@github-actions github-actions bot added the part:docs Affects the documentation label Feb 16, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates LatestValueCache to take ownership of its Receiver and ensure the receiver is closed when the cache is stopped, preventing unused receivers from remaining attached to channels.

Changes:

  • Document that LatestValueCache takes ownership of the receiver and will close it on stop.
  • Call self._receiver.close() from LatestValueCache.stop().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@@ -109,6 +112,7 @@ async def _run(self) -> None:

async def stop(self) -> None:
"""Stop the cache."""
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

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

stop() now closes the underlying receiver, but stop()'s docstring still only says "Stop the cache". Please update the method docstring to mention that it also closes the owned receiver, so callers don't accidentally expect to keep using it after stopping.

Suggested change
"""Stop the cache."""
"""Stop the cache and close the owned receiver.
After this method is called, the underlying receiver is closed and
must not be used anymore.
"""

Copilot uses AI. Check for mistakes.
Comment on lines +56 to +58

Takes ownership of the receiver. When the cache is stopped, the receiver
will be closed.
Copy link

Copilot AI Feb 16, 2026

Choose a reason for hiding this comment

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

Since this is a behavior/ownership change, consider also updating the module/package-level documentation for LatestValueCache (e.g., the module docstring and/or the entry in frequenz.channels.__init__ docs) so users reading the top-level docs learn that stop() closes the receiver.

Copilot uses AI. Check for mistakes.
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
@shsms shsms force-pushed the improve-latest-value-cache-cleanup branch from 0acba17 to 67d58a8 Compare February 16, 2026 14:14
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
@shsms shsms force-pushed the improve-latest-value-cache-cleanup branch from 2d75df7 to a812c29 Compare February 16, 2026 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

part:docs Affects the documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant