Remove outdated lock from ParseRawHeaderValues #54130
Conversation
This lock existed to protect HttpClient.DefaultRequestHeaders. While headers collections aren't meant to be thread-safe, out of necessity the design of DefaultRequestHeaders required this lock be in place, with multiple concurrent requests all potentially enumerating DefaultRequestHeaders in order to copy its contents into the outgoing request. Such enumeration could trigger parsing, which could trigger the same object to be mutated from multiple threads to store the parsed result. But as of dotnet#49673, we no longer force parsing of the DefaultRequestHeaders, instead preferring to just transfer everything over as-is. With that, there shouldn't be any concurrent mutation of these objects (and if there is, it's user error doing their own concurrent enumeration of a header collection).
ghost
added
the
|
Tagging subscribers to this area: @dotnet/ncl Issue DetailsThis lock existed to protect HttpClient.DefaultRequestHeaders. While headers collections aren't meant to be thread-safe, out of necessity the design of DefaultRequestHeaders required this lock be in place, with multiple concurrent requests all potentially enumerating DefaultRequestHeaders in order to copy its contents into the outgoing request. Such enumeration could trigger parsing, which could trigger the same object to be mutated from multiple threads to store the parsed result, hence the lock. But as of #49673, we no longer force parsing of the DefaultRequestHeaders, instead preferring to just transfer everything over as-is. With that, there shouldn't be any concurrent mutation of these objects (and if there is, it's user error doing their own concurrent enumeration of a header collection). Please quadruple check me on this 😄
|
|
Note from #55898 (comment): This lock was preventing side-effects of reading the collection from multiple threads. For example, we may have allocated We may have modified the internal dictionary, but |
4 participants
This lock existed to protect HttpClient.DefaultRequestHeaders. While headers collections aren't meant to be thread-safe, out of necessity the design of DefaultRequestHeaders required this lock be in place, with multiple concurrent requests all potentially enumerating DefaultRequestHeaders in order to copy its contents into the outgoing request. Such enumeration could trigger parsing, which could trigger the same object to be mutated from multiple threads to store the parsed result, hence the lock. But as of #49673, we no longer force parsing of the DefaultRequestHeaders, instead preferring to just transfer everything over as-is. With that, there shouldn't be any concurrent mutation of these objects (and if there is, it's user error doing their own concurrent enumeration of a header collection).
Please quadruple check me on this 😄
cc: @geoffkizer, @scalablecory