Skip to content

DeepCloneFrom / IEnumerable.DeepClone share dictionary values with the source when it is not an IDictionary (e.g. IReadOnlyDictionary, LINQ over a dictionary) #82

Description

@matt-edmondson

What's wrong

When the source of a clone is a sequence of KeyValuePair<TKey, TValue> but not statically an IDictionary<TKey, TValue>, overload resolution picks the element-wise overloads with T = KeyValuePair<TKey, TValue>:

  • DeepCloneFrom<T>(this ICollection<T> dest, IEnumerable<T> source) (DeepClone/DeepCloneContainerExtensions.cs:36)
  • DeepClone<T>(this IEnumerable<T> source) (DeepCloneContainerExtensions.cs:116)

Both clone each element through the private helper at DeepCloneContainerExtensions.cs:97:

private static T DeepClone<T>(T source) => source == null ? default! : source is IDeepCloneable cloneable ? (T)cloneable.DeepClone() : source;

A KeyValuePair<,> never implements IDeepCloneable, so the pair is copied as-is and the key and value objects are shared with the source. Nothing errors.

Repro

var src = new Dictionary<string, Obj> { ["a"] = new Obj() };   // Obj : DeepCloneable<Obj>

var dest = new Dictionary<string, Obj>();
dest.DeepCloneFrom((IReadOnlyDictionary<string, Obj>)src);
ReferenceEquals(dest["a"], src["a"]);            // True  — expected False

var list = src.Where(_ => true).DeepClone().ToList();
ReferenceEquals(list[0].Value, src["a"]);        // True  — expected False

The same thing happens with any IEnumerable<KeyValuePair<,>> source, such as an IReadOnlyDictionary, ImmutableDictionary exposed through an interface, or a LINQ projection over a dictionary.

Why it matters

The caller asked for a deep clone and got one back without any error, but the values are the original instances. Mutating the "clone" then mutates the source. This is the kind of aliasing bug the library exists to prevent. The IDictionary overload of DeepCloneFrom does clone both key and value, so the result depends on the static type the caller happened to hold.

Suggested fix

Either of these works:

Acceptance: the repro above returns False in both cases, with a regression test for each of IReadOnlyDictionary and a LINQ-filtered source.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

bugSomething isn't workingreadyFully specified; implement as written

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions