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

Add a check to ShouldUseSameServiceProvider even when the extension has no options #26111

Merged
merged 1 commit into from
Sep 20, 2021

Conversation

AndriySvyryd
Copy link
Member

@AndriySvyryd AndriySvyryd commented Sep 20, 2021

Fixes #26109

Description

Currently different optionless extensions return true in ShouldUseSameServiceProvider this could still cause a collision if the hash codes of their types happen to be the same.

Customer impact

Without this an incompatible cached service provider could be resolved, causing a DI exception.

How found

PR review

Regression

No, this method is new in 6.0.

Testing

No additional tests as regression tests for this wouldn't be meaningful due to the nature of hash collisions.

Risk

Low, the new method is only used in a single place.

@AndriySvyryd
Copy link
Member Author

@Pilchie For approval

@ajcvickers
Copy link
Member

Good catch!

@Pilchie
Copy link
Member

Pilchie commented Sep 20, 2021

the hash codes of their types happen to be the same.

Is this the hash code of the System.Type instance? Is it even possible for them to be the same?

Anyway, approved for EFCore 6.0

@AndriySvyryd
Copy link
Member Author

Is this the hash code of the System.Type instance?

Yes

Is it even possible for them to be the same?

In practice, probably not.
But since it's not guaranteed as part of the contract better not to rely on it.

@AndriySvyryd AndriySvyryd changed the title [6.0] Add a check to ShouldUseSameServiceProvider even when the extension has no options Add a check to ShouldUseSameServiceProvider even when the extension has no options Sep 20, 2021
@AndriySvyryd AndriySvyryd merged commit 501c60a into release/6.0 Sep 20, 2021
@AndriySvyryd AndriySvyryd deleted the Issue26109 branch September 20, 2021 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update ShouldUseSameServiceProvider implementation for empty options
3 participants