Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: make reload::Subscriber compatible with per-subscriber filters #2965

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mladedav
Copy link
Contributor

@mladedav mladedav commented May 9, 2024

Motivation

If the reload::Subscriber was used with a filtered Subscribe, the filters panicked with debug_assertions because nothing cleared interests of the filters. This is because on_subscribe was not called and so the Registry didn't know there were filters involved.

See the new test for an example of what currently panics on master.

downcast_raw also did not special-case the magic filter marker so even if the inner subscriber was filtered, other layers could never find out.

Solution

I've added an implementation for on_subscribe in reload::Subscriber so that filters are properly registered within the Registry.

I've also added an unsafe implementation to downcasting to special-case the magic marker. I've tried to make sure for this to be completely safe even if the caller decides to dereference the returned pointer. This implementation is needed so that Layered and others can tell whether the reload::Subscriber uses psf or not.

@mladedav mladedav requested review from hawkw, davidbarsky and a team as code owners May 9, 2024 21:39
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.

None yet

1 participant