Call TypeDescriptor.GetProperties before calling Refresh in ReflectionCachesUpdateHandler#51604
Call TypeDescriptor.GetProperties before calling Refresh in ReflectionCachesUpdateHandler#51604stephentoub wants to merge 2 commits intomainfrom
Conversation
ghost
added
the
|
Tagging subscribers to this area: @safern Issue DetailsTry to fix #51588 According to the docs: Counterintuitive, but ok. cc: @jeffhandley, @safern
|
jeffhandley
left a comment
jeffhandley
left a comment
There was a problem hiding this comment.
Thanks! Any idea why it was only failing on macOS?
My guess is the failures are happening when this test runs concurrently with some other test, and without the GetProperties call, Refresh ends up not being thread-safe. At least that's my very limited understanding from the docs and a quick perusal of the source. If that's true, then this isn't limited to just macOS, and that's simply the only place we've seen it so far. |
|
Seems like a reasonable hunch. Thanks for addressing it so swiftly! |
…nCachesUpdateHandler
stephentoub
force-pushed
the
fixtcreflectioncache
branch
from
bcfca50 to
592e90e
Compare
|
Looks like it's also failed in Linux now so not Mac specific |
|
And also not fixed by this change. It's possible TypeDescriptor.Refresh simply isn't thread-safe, violating rules around statics in .NET. I'll need to look deeper. |
TypeDescriptor.Refresh isn't thread-safe, trying to enumerate a hashtable that's concurrently mutated, leading to a concurrent modification exceptions. We'll need to decide if we're ok with the limitation or if we need to do surgery on TypeDescriptor to fix it.
5 participants
Try to fix #51588
According to the docs:
"Before you make a call to the Refresh method to clear the cache, you need to call the GetProperties method for the specific assembly to cache the information first."
Counterintuitive, but ok.
cc: @jeffhandley, @safern