Shouldn't TypeDictionary be thread-safe?

[AArnott]

I know it's just in the Samples area, but as sample tend to be copied for production code, shouldn't TypeDictionary be thread-safe?

It looks like maybe it doesn't need to be for present purposes because the existing uses create a new instance of Builder classes so TypeDictionary is never shared across threads. But that means multiple type graphs that overlap will produce redundant serializers, if I understand correctly.

[eiriktsarpalis]

It isn't intended to be thread-safe, no. The only differentiator for this type over a regular Dictionary<Type, object> is that it facilitates delayed serializer construction in cases where you have recursive types. As such it can contain incomplete values that will throw an exception if you try to exercise them while their type graph is still begin traversed.

But that means multiple type graphs that overlap will produce redundant serializers, if I understand correctly.

That's correct. There's no real solution I know of other than taking a lock on the dictionary while a particular type graph is being built. That's probably acceptable given it's a one-time cost per type (System.Text.Json also does it this way). Another alternative is keeping a global ConcurrentDictionary<Type, object> that you use to optimistically commit all values from a local TypeDictionary.

[eiriktsarpalis]

Another alternative is keeping a global ConcurrentDictionary<Type, object> that you use to optimistically commit all values from a local TypeDictionary.

FWIW the F# variant of the library defines a thread-safe TypeCache type that can be used to create local "generation contexts" that commit their results back to the global cache once disposed:

https://github.com/eiriktsarpalis/TypeShape/blob/ddc64e8d8f8254a172e320b46794b5740c404780/samples/TypeShape.Samples/equality-comparer.fs#L14-L15

Maybe it's useful to port this pattern to C# as well.

[AArnott]

I suppose making that collection thread-safe isn't a good solution because if multiple graphs were being constructed concurrently, there's no guarantee that one type isn't requested and activated before it's "ready".
I'm thinking about a large graph that includes a recursive type, so it sticks the recursive type into the graph provisionally, and keeps working. In the meantime another graph is built that needs that type, finds it in the collection, and quickly exits claiming it's done, yet the type isn't fully assembled, and an exception is thrown.