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 ConditionalWeakTable the same as in CoreRT #5133

Closed
mjp41 opened this issue Feb 17, 2016 · 5 comments
Closed

Make ConditionalWeakTable the same as in CoreRT #5133

mjp41 opened this issue Feb 17, 2016 · 5 comments
Assignees
Milestone

Comments

@mjp41
Copy link
Member

mjp41 commented Feb 17, 2016

There are several concurrency issues in the ConditionalWeakTable, these have been fixed in CoreRT's ConditionWeakTable.

The redesign in CoreRT should be integrated into CoreCLR.

@stephentoub
Copy link
Member

It'd be nice to bring over the CoreRT design for other reasons... IIRC, the CoreRT ConditionalWeakTable provides for lock-free reads, whereas the CoreCLR one requires a lock.

@mjp41
Copy link
Member Author

mjp41 commented Feb 17, 2016

I think it is attempting to do lock-free reads here:

https://github.com/dotnet/coreclr/blob/master/src/mscorlib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs#L238

But I don't think that is correct. The CoreRT version does this correctly.

@stephentoub
Copy link
Member

I think it is attempting to do lock-free reads here

No, TryGetValue takes a lock:
https://github.com/dotnet/coreclr/blob/master/src/mscorlib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs#L109

@mjp41
Copy link
Member Author

mjp41 commented Feb 17, 2016

Sorry, you are correct. I had spent too much time starring at the other. Thank you. I think the version is correct, so I retract my comments on concurrency.

But making them the same would be good. I am planning to address an issue in CoreRT for the ConditionalWeakTable.

@mjp41 mjp41 changed the title Concurrency issues in ConditionalWeakTable Make ConditionalWeakTable the same as in CoreRT Feb 17, 2016
@stephentoub
Copy link
Member

I'll take a look at porting the corert lock-free reads support back to CoreCLR...

@stephentoub stephentoub self-assigned this Dec 7, 2016
@msftgits msftgits transferred this issue from dotnet/coreclr Jan 30, 2020
@msftgits msftgits added this to the Future milestone Jan 30, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Jan 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants