Newtonsoft.Json
Newtonsoft.Json copied to clipboard
DefaultReferenceResolver Concurrency Fix
In case you won't accept #1393 this is the easy fix to some of the concurrency issues.
Fixes #870 and #1452
Any update on this? Ran into this issue in https://github.com/akkadotnet/akka.net/pull/5197
This type isn't designed to have thread-safety.
Even if the id was incremented with Interlocked, the underlying collections aren't thread-safe. And if the collections were made threadsafe then performance would probably take a big hit because all serialization shares the same collections and there will be a ton of lock contention.
It seems the underlying BidirectionalDictionary is specific to the serialization context and thus does not need to be thread safe as it should only be accessed from a single thread whereas the DefaultReferenceResolver itself is shared across serialization calls thus users have experienced these concurrency issues.
You yourself said JsonSerializer is thread-safe at https://stackoverflow.com/questions/36186276/is-the-json-net-jsonserializer-threadsafe so it seems the intent would be for the DefaultReferenceResolver to be thread-safe also since it's used by JsonSerializer but that's not true when using references as indicated in my answer to that question. This small change should fix the concurrency issue but I still think this type should either avoid being stateful by pushing the reference count into the context or else creating a new ReferenceResolver for each serialization call as is proposed in #1393.
Yeah the idea that the serializer itself isn't / can't be thread-safe seems crazy to me. Move the state to the context or the call, not the callsite itself.