jackson-databind
jackson-databind copied to clipboard
Remove unnecessary external synchronization from `SerializerCache` access of `_sharedMap`
(note: off-shoot of #4430 )
Due to historical reasons, SerializerCache uses synchronized around all access to LookupCache _sharedMap. This is not necessary as LookupCache implementations must implement thread-safe access.
Let's remove these statements around access.
But just in case do it only for 2.18, not 2.17 patch release.
On second thought... while access in general should not require synchronized, it looks like there might be one big wrinkle -- resolution of cyclic dependencies.
Looking at this:
public void addAndResolveNonTypedSerializer(Class<?> type, JsonSerializer<Object> ser,
SerializerProvider provider)
throws JsonMappingException
{
synchronized (this) {
if (_sharedMap.put(new TypeKey(type, false), ser) == null) {
_readOnlyMap.set(null);
}
// Need resolution to handle cyclic POJO type dependencies
/* 14-May-2011, tatu: Resolving needs to be done in synchronized manner;
* this because while we do need to register instance first, we also must
* keep lock until resolution is complete.
*/
if (ser instanceof ResolvableSerializer) {
((ResolvableSerializer) ser).resolve(provider);
}
}
}
I realize that not only does resolve() logic need to be synced for instance, that instance should NOT be exposed before completion via _sharedMap either.
So it may actually be necessary to have those synchronized blocks just for this reason: not to protect LookupCache itself (which is thread-safe) but partially initialized BeanSerializers.
Given this I will not proceed with this immediately.
Not safe to remove, closing.