Skip to content

Prune dead observers on registration#751

Open
wdcui wants to merge 1 commit intomainfrom
wdcui/pr3a-prune-observers
Open

Prune dead observers on registration#751
wdcui wants to merge 1 commit intomainfrom
wdcui/pr3a-prune-observers

Conversation

@wdcui
Copy link
Copy Markdown
Member

@wdcui wdcui commented Apr 7, 2026

Prune stale Weak references from the observer list every time a new observer is registered, preventing unbounded growth when observers are dropped without explicit removal.

Add prune_dead_observers() to Subject, called during register_observer()
to eagerly clean up stale weak references. Previously dead observers were
only cleaned up during notify_observers(), which could leave orphaned
entries indefinitely if notifications were infrequent.

Includes a test verifying that registering a new observer prunes stale
entries from a previously dropped observer.
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 7, 2026

🤖 SemverChecks 🤖 No breaking API changes detected

Note: this does not mean API is unchanged, or even that there are no breaking changes; simply, none of the detections triggered.

Copy link
Copy Markdown
Member

@jaybosamiya-ms jaybosamiya-ms left a comment

Choose a reason for hiding this comment

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

Generally looks fine to me, adding @CvvT as a reviewer due to greater familiarity with the event system for final approval.

Minor nit: we don't need the # Summary in the PR description, or that it was split from #743. It also does not need to document that it adds a unit test. I'll update the PR description, but just making a note.

@jaybosamiya-ms jaybosamiya-ms requested a review from CvvT April 11, 2026 00:23
@jaybosamiya-ms jaybosamiya-ms added the expmt:shadow-kiln Tag to quickly find the different PRs as part of the "shadow kiln" experiment. label Apr 11, 2026
Copy link
Copy Markdown
Contributor

@CvvT CvvT left a comment

Choose a reason for hiding this comment

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

This PR looks good to me, but I am not sure whether this is necessary. Did you encounter any issues because of too many stale observers?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

expmt:shadow-kiln Tag to quickly find the different PRs as part of the "shadow kiln" experiment.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants