You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
The text was updated successfully, but these errors were encountered:
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.
(note: off-shoot of #4430 )
Due to historical reasons,
SerializerCache
usessynchronized
around all access toLookupCache _sharedMap
. This is not necessary asLookupCache
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.
The text was updated successfully, but these errors were encountered: