dotnet / dotnet-api-docs

.NET API reference documentation (.NET 5+, .NET Core, .NET Framework)
https://docs.microsoft.com/dotnet/api/
Other
735 stars 1.57k forks source link

ReadOnlyDictionary Values and Keys collections are out of sync with KeyValuePairs #8338

Open ante-maric opened 2 years ago

ante-maric commented 2 years ago

Description

When underlying dictionary passed to a constructor of ReadOnlyDictionary is ConcurrentDictionary, all KeyValuePairs that are added to the ConcurrentDictionary after ReadOnlyDictionary is created are not visible through ReadOnlyDictionary.Values and ReadOnlyDictionary.Keys collection. However, ReadOnlyDictionary indexer and ContainsKey work correctly.

All of this is not a problem if regular Dictionary is used.

Reproduction Steps

        var dictionary = new ConcurrentDictionary<string, int>();
        dictionary["1"] = 1;
        dictionary["2"] = 2;
        dictionary["3"] = 3;
        var readOnlyDictionary = new ReadOnlyDictionary<string, int>(dictionary);
        readOnlyDictionary.Values.Count.ShouldBe(dictionary.Count); // ok, count is 3

        dictionary["4"] = 4;
        readOnlyDictionary.ContainsKey("4").ShouldBeTrue(); // ok, key is present
        readOnlyDictionary["4"].ShouldBe(dictionary["4"]); // ok, value is 4
        readOnlyDictionary.Values.Count.ShouldBe(dictionary.Count); // fails, count is 3

Expected behavior

Every KeyValuePair that is retrievable by ReadOnlyDictionary indexer. ContainsKey, TryGetValue... should also be present in Values and Keys collections. Everything should be in sync with underlying ConcurrentDictionary.

Actual behavior

ReadOnlyDictionary's Values and Keys collections do not contain items that are added to underlying dictionary after creation of ReadOnlyDictionary if underlying dictionary is of type ConcurrentDictionary.

Regression?

No response

Known Workarounds

No response

Configuration

.NET version is 6.0.400 running on Windows 10 21H1.

Other information

No response

ghost commented 2 years ago

Tagging subscribers to this area: @dotnet/area-system-collections See info in area-owners.md if you want to be subscribed.

Issue Details
### Description When underlying dictionary passed to a constructor of ReadOnlyDictionary is ConcurrentDictionary, all KeyValuePairs that are added to the ConcurrentDictionary after ReadOnlyDictionary is created are not visible through ReadOnlyDictionary.Values and ReadOnlyDictionary.Keys collection. However, ReadOnlyDictionary indexer and ContainsKey work correctly. All of this is not a problem if regular Dictionary is used. ### Reproduction Steps ``` var dictionary = new ConcurrentDictionary(); dictionary["1"] = 1; dictionary["2"] = 2; dictionary["3"] = 3; var readOnlyDictionary = new ReadOnlyDictionary(dictionary); readOnlyDictionary.Values.Count.ShouldBe(dictionary.Count); // ok, count is 3 dictionary["4"] = 4; readOnlyDictionary.ContainsKey("4").ShouldBeTrue(); // ok, key is present readOnlyDictionary["4"].ShouldBe(dictionary["4"]); // ok, value is 4 readOnlyDictionary.Values.Count.ShouldBe(dictionary.Count); // fails, count is 3 ``` ### Expected behavior Every KeyValuePair that is retrievable by ReadOnlyDictionary indexer. ContainsKey, TryGetValue... should also be present in Values and Keys collections. Everything should be in sync with underlying ConcurrentDictionary. ### Actual behavior ReadOnlyDictionary's Values and Keys collections do not contain items that are added to underlying dictionary after creation of ReadOnlyDictionary if underlying dictionary is of type ConcurrentDictionary. ### Regression? _No response_ ### Known Workarounds _No response_ ### Configuration .NET version is 6.0.400 running on Windows 10 21H1. ### Other information _No response_
Author: ante-maric
Assignees: -
Labels: `area-System.Collections`
Milestone: -
svick commented 2 years ago

This is not really about ReadOnlyDictionary, but about the behavior of ConcurrentDictionary.Values (and Keys):

var dictionary = new ConcurrentDictionary<string, int>();
dictionary["1"] = 1;
dictionary["2"] = 2;
dictionary["3"] = 3;
var values = dictionary.Values;
values.Count.ShouldBe(dictionary.Count); // ok, count is 3

dictionary["4"] = 4;
dictionary.ContainsKey("4").ShouldBeTrue(); // ok, key is present
values.Count.ShouldBe(dictionary.Count); // fails, count is 3

This happens because accessing Values creates a snapshot of the dictionary (and ReadOnlyDictionary only accesses Values of the wrapped dictionary once).

I suspect that this is an intentional design decision that's not going to change. But at least it should be documented.

ante-maric commented 2 years ago

Since accessing the Values only once is the behavior of ReadOnlyDictionary, I would argue this is about ReadOnlyDictionary. You can't observe this issue with ConcurrentDictionary alone. But we could agree it's potato-potato. What I would suggest that warning should be part of ReadOnlyDictionary documentation and adding the same to ConcurrentDictionary documentation would be a bonus, maximizing the chance for that to be seen by those who really need to.

eiriktsarpalis commented 2 years ago

Since accessing the Values only once is the behavior of ReadOnlyDictionary, I would argue this is about ReadOnlyDictionary.

Generally speaking, most IDictionary.Keys/Values collection implementations are kept in sync with their parent dictionary, so I suspect this is more an issue of ConcurrentDictionary being special rather than ReadOnlyDictionary requiring documentation.