Potential race conditions in DataContractSerialization

[jkotas]

From dotnet/coreclr#17076

• This is buggy: https://github.com/dotnet/corefx/blob/619dbe6503a2de2afa27122cb34716d9ff6906ff/src/System.Runtime.Serialization.Xml/tests/SerializationTestTypes/DataContractResolverLibrary.cs#L16-L34. But it's in tests, and it would only manifest if tests that were using this type were to run concurrently; it looks like all tests that use it are in the same xunit class, so they won't run concurrently. Still, it's probably worth fixing just to avoid issues in the future should usage here be extended.
• Just based on brief code inspection, these look problematic: https://github.com/dotnet/corefx/blob/619dbe6503a2de2afa27122cb34716d9ff6906ff/src/Common/tests/System/Runtime/Serialization/Utils.cs#L38-L39. Those dictionaries are read/written without locks from various static helper methods. These could be in the same camp as above, though, where they're "safe" only because all tests that use them end up getting serialized.
• This looks dangerous: https://github.com/dotnet/corefx/blob/bb399884797d73f5401c637ed861745b97f0fd92/src/System.Private.DataContractSerialization/src/System/Runtime/Serialization/DataContract.cs#L30-L36. But I also can't find any references to GetDataContracts() elsewhere in corefx, so usage isn't clear. And enough care was taken with other shared dictionaries elsewhere in the file that presumably the author here knew what they were doing.
• This looks like a product bug, and one that this change could potentially catch (cc: @shmao). https://github.com/dotnet/corefx/blob/bb399884797d73f5401c637ed861745b97f0fd92/src/System.Private.DataContractSerialization/src/System/Runtime/Serialization/ExtensionDataReader.cs#L487-L496. But again I don't have enough context as to usage, and I don't know if we have sufficient tests in place to help this change catch it. Similarly it's suspicious that there are multiple static methods that access another dictionary on this type, writing https://github.com/dotnet/corefx/blob/bb399884797d73f5401c637ed861745b97f0fd92/src/System.Private.DataContractSerialization/src/System/Runtime/Serialization/ExtensionDataReader.cs#L501-L505 and reading https://github.com/dotnet/corefx/blob/bb399884797d73f5401c637ed861745b97f0fd92/src/System.Private.DataContractSerialization/src/System/Runtime/Serialization/ExtensionDataReader.cs#L209 and without explicit synchronization, at least at this level.

[jiayi11]

Hi @jkotas, the third one (pasted below) is guaranteed safe because it is used in pre-generated serializer on UWP only. I will submit a pr to fix the other issues later. Thanks!

• This looks dangerous: https://github.com/dotnet/corefx/blob/bb399884797d73f5401c637ed861745b97f0fd92/src/System.Private.DataContractSerialization/src/System/Runtime/Serialization/DataContract.cs#L30-L36.

[karelz]

Fixing milestone, the change went into master (3.0) branch, not into 2.2.