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

NodeStateTable.GetModifiedItemAndState does not guard against a throwing .Equals method #76765

Open
chsienki opened this issue Jan 15, 2025 · 0 comments · May be fixed by #76769
Open

NodeStateTable.GetModifiedItemAndState does not guard against a throwing .Equals method #76765

chsienki opened this issue Jan 15, 2025 · 0 comments · May be fixed by #76769
Assignees
Milestone

Comments

@chsienki
Copy link
Contributor

chsienki commented Jan 15, 2025

When the comparer used here:

private static (T chosen, EntryState state, bool chosePrevious) GetModifiedItemAndState(T previous, T replacement, IEqualityComparer<T> comparer)
{
// when comparing an item to check if its modified we explicitly cache the *previous* item in the case where its
// considered to be equal. This ensures that subsequent comparisons are stable across future generation passes.
return comparer.Equals(previous, replacement)
? (previous, EntryState.Cached, chosePrevious: true)
: (replacement, EntryState.Modified, chosePrevious: false);
}

is not a user supplied comparer, and the call to T.Equals() throws, we don't catch the exception and convert into a UserFunctionException. This can cause a generator to bring the hosting process down. We should always wrap default comparers in a user wrapped user comparer to ensure we still do the translation.

See here for an example: microsoft/CsWinRT#1897

@chsienki chsienki self-assigned this Jan 15, 2025
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Issues and PRs which have not yet been triaged by a lead label Jan 15, 2025
@jaredpar jaredpar added this to the 17.14 milestone Jan 15, 2025
@jaredpar jaredpar removed the untriaged Issues and PRs which have not yet been triaged by a lead label Jan 15, 2025
@jaredpar jaredpar modified the milestones: 17.14, 17.13 Jan 15, 2025
@chsienki chsienki linked a pull request Jan 16, 2025 that will close this issue
@chsienki chsienki changed the title NodeStateTable.GetModifiedItemAndState does not guard against a throwing comparer. NodeStateTable.GetModifiedItemAndState does not guard against a throwing .Equals method Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants