Use ConcurrentDictionary to avoid threading issues#104407
Use ConcurrentDictionary to avoid threading issues#104407steveharter merged 7 commits intodotnet:mainfrom
Conversation
...ibraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/PropertyDescriptor.cs
Outdated
Show resolved
Hide resolved
AustinWise
left a comment
AustinWise
left a comment
There was a problem hiding this comment.
You could potentially use features of the ConcurrentDictionary to avoid the need for a lock. See this gist: https://gist.github.com/AustinWise/92f293463124cdbf95e702946502b636
...ibraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/PropertyDescriptor.cs
Show resolved
Hide resolved
| _valueChangedHandlers[component] = h; | ||
| } | ||
| else | ||
| lock (SyncObject) |
There was a problem hiding this comment.
| lock (SyncObject) | |
| while (true) | |
| { | |
| EventHandler? existingHandler; | |
| if (!_valueChangedHandlers.TryGetValue(component, out existingHandler)) | |
| { | |
| break; | |
| } | |
| EventHandler? replacementHandler = (EventHandler?)Delegate.Remove(existingHandler, handler); | |
| if (replacementHandler is null) | |
| { | |
| if (_valueChangedHandlers.TryRemove(new KeyValuePair<object, EventHandler?>(component, existingHandler))) | |
| { | |
| break; | |
| } | |
| } | |
| else | |
| { | |
| if (_valueChangedHandlers.TryUpdate(component, replacementHandler, existingHandler)) | |
| { | |
| break; | |
| } | |
| } | |
| } |
I intend for this to replace the entire lock block, but GitHub won't let me make that suggestion. See here for what it should look like: https://gist.github.com/AustinWise/92f293463124cdbf95e702946502b636#file-propertydescriptor-cs-L62
There was a problem hiding this comment.
I think using the lock as-is is more straightforward. Also, the Remove is not likely to be used as often.
...ibraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/PropertyDescriptor.cs
Outdated
Show resolved
Hide resolved
This reverts commit 8fc2719.
...ibraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/PropertyDescriptor.cs
Outdated
Show resolved
Hide resolved
...tem.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs
Outdated
Show resolved
Hide resolved
6 participants
Address various threading issues including:
Fixes #104221
Fixes #103823
Fixes #102136
Local testing included running all tests in a loop 300x in both Release and Debug builds.
Linking here other threading issues fixed in this area fixed during 9.0:
#30024
#92394
#97179
#103265