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

Make TypeDescriptor thread safe with custom providers and minimize lock use #92548

Closed
wants to merge 8 commits into from

Conversation

karakasa
Copy link
Contributor

@karakasa karakasa commented Sep 24, 2023

Summary

Fix #92394

Similar to #92521 but the PR takes a more aggressive approach. The PR creates specialized versions of AddProvider and NodeFor(Type) for use in CheckDefaultProvider to minimize lock. Though the side effects are not fully reviewed due to control flow change.

Risk

The performance should be close to, or better than, the before-PR implementation because the lock statement doesn't change.

The PR might introduce bugs as it puts s_defaultProviders[type] = null after AddProvider.

Originally a race condition exists in `CheckDefaultProvider` and leads
to wrong results when many methods are called simultaneously.

The PR fixes that by extending the lock statement.

Fix dotnet#92934
to wrong results when many methods are called simultaneously.

The PR fixes that by extending the lock statement.

Fix dotnet#92394
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Sep 24, 2023
@ghost
Copy link

ghost commented Sep 24, 2023

Tagging subscribers to this area: @dotnet/area-system-componentmodel
See info in area-owners.md if you want to be subscribed.

Issue Details

Fix #92394

Similar to #92521 but the PR takes a more aggressive approach. The PR creates specialized versions of AddProvider and NodeFor(Type) for use in CheckDefaultProvider to minimize lock. Though the side effects are not fully reviewed.

Author: karakasa
Assignees: -
Labels:

area-System.ComponentModel

Milestone: -

@karakasa
Copy link
Contributor Author

karakasa commented Sep 24, 2023

Sometimes the PR causes stack overflow. Reject.

@karakasa karakasa closed this Sep 24, 2023
@karakasa karakasa deleted the issue-92394-fewer-lock branch September 27, 2023 16:35
@ghost ghost locked as resolved and limited conversation to collaborators Oct 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.ComponentModel community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeDescriptor.GetProperties(object instance) is not thread-safe
1 participant