Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Null-safe equality check #1292

Closed
wants to merge 1 commit into from
Closed

Conversation

hipatel7
Copy link

@hipatel7 hipatel7 commented Jul 8, 2016

This equality check needs to be null-safe. It appears that getSelfReferencedType() may not be always non-null. Due to this, I have seen null pointer exceptions thrown from ResolvedRecursiveType.equals() .

Here's one example where object of type ResolvedRecursiveType is created which could return null for getSelfReferencedType() calls. :

                // In TypeFactory.java
                // Self-reference: needs special handling, then...
                ResolvedRecursiveType selfRef = new ResolvedRecursiveType(rawType, EMPTY_BINDINGS);
                prev.addSelfReference(selfRef);
                return selfRef;

@cowtowncoder
Copy link
Member

Before adding such a check, it would be good to be able reproduce the problem: I don't want to be hiding issues where null is a result of something else being done wrong. I understand that there may be a case where reference could be missing temporarily, but not convinced this is a valid sate yet.

@hipatel7
Copy link
Author

hipatel7 commented Jul 8, 2016

Agreeing to your point there. However, I've started getting these errors when I use version 2.8.0 of jackson-databind.


java.lang.NullPointerException
    at com.fasterxml.jackson.databind.type.ResolvedRecursiveType.equals(ResolvedRecursiveType.java:103)
    at com.fasterxml.jackson.databind.type.TypeBindings$AsKey.equals(TypeBindings.java:458)
    at java.util.concurrent.ConcurrentHashMap.get(ConcurrentHashMap.java:940)
    at com.fasterxml.jackson.databind.util.LRUMap.get(LRUMap.java:68)
    at com.fasterxml.jackson.databind.type.TypeFactory._fromClass(TypeFactory.java:1211)
    at com.fasterxml.jackson.databind.type.TypeFactory._fromParamType(TypeFactory.java:1384)
    at com.fasterxml.jackson.databind.type.TypeFactory._fromAny(TypeFactory.java:1154)
    at com.fasterxml.jackson.databind.type.TypeFactory._resolveSuperInterfaces(TypeFactory.java:1298)
    at com.fasterxml.jackson.databind.type.TypeFactory._fromClass(TypeFactory.java:1243)
    at com.fasterxml.jackson.databind.type.TypeFactory._fromAny(TypeFactory.java:1150)
    at com.fasterxml.jackson.databind.type.TypeFactory._resolveSuperInterfaces(TypeFactory.java:1298)
    at com.fasterxml.jackson.databind.type.TypeFactory._fromClass(TypeFactory.java:1247)
    at com.fasterxml.jackson.databind.type.TypeFactory._fromAny(TypeFactory.java:1150)
    at com.fasterxml.jackson.databind.type.TypeFactory.constructType(TypeFactory.java:622)
    at com.fasterxml.jackson.databind.introspect.AnnotatedClass.resolveType(AnnotatedClass.java:228)
    at com.fasterxml.jackson.databind.introspect.AnnotatedMethod.getParameterType(AnnotatedMethod.java:157)
    at com.fasterxml.jackson.databind.introspect.AnnotatedWithParams.getParameter(AnnotatedWithParams.java:105)
    at com.fasterxml.jackson.databind.introspect.POJOPropertiesCollector._addCreators(POJOPropertiesCollector.java:457)
    at com.fasterxml.jackson.databind.introspect.POJOPropertiesCollector.collectAll(POJOPropertiesCollector.java:303)
    at com.fasterxml.jackson.databind.introspect.POJOPropertiesCollector.getJsonValueMethod(POJOPropertiesCollector.java:172)
    at com.fasterxml.jackson.databind.introspect.BasicBeanDescription.findJsonValueMethod(BasicBeanDescription.java:224)
    at com.fasterxml.jackson.databind.ser.BasicSerializerFactory.findSerializerByAnnotations(BasicSerializerFactory.java:350)
    at com.fasterxml.jackson.databind.ser.BeanSerializerFactory._createSerializer2(BeanSerializerFactory.java:210)
    at com.fasterxml.jackson.databind.ser.BeanSerializerFactory.createSerializer(BeanSerializerFactory.java:159)
    at com.fasterxml.jackson.databind.SerializerProvider._createUntypedSerializer(SerializerProvider.java:1272)
    at com.fasterxml.jackson.databind.SerializerProvider._createAndCacheUntypedSerializer(SerializerProvider.java:1222)
    at com.fasterxml.jackson.databind.SerializerProvider.findValueSerializer(SerializerProvider.java:499)
    at com.fasterxml.jackson.databind.SerializerProvider.findTypedValueSerializer(SerializerProvider.java:697)
    at com.fasterxml.jackson.databind.ser.DefaultSerializerProvider.serializeValue(DefaultSerializerProvider.java:270)
    at com.fasterxml.jackson.databind.ObjectMapper._configAndWriteValue(ObjectMapper.java:3672)
    at com.fasterxml.jackson.databind.ObjectMapper.writeValue(ObjectMapper.java:3030)

@cowtowncoder
Copy link
Member

cowtowncoder commented Jul 8, 2016

@hipatel7 Ok. This is bit worrisome also from perspective that perhaps it could also cause incorrect behavior when exception would otherwise be thrown... do you think this would be due to concurrency (something accessing half-resolved type), or not? If it is, reproduction could be challenging.

Another question on version: which version are upgrading from? 2.7 or earlier? If 2.7, I wonder if this could be due to caching that was to be added; and if so, whether I have to re-consider caching of generic types.

@hipatel7
Copy link
Author

hipatel7 commented Jul 8, 2016

I tested it, up to 2.7.5 it works. In 2.8.0, I see these errors.

This isn't related to concurrency. I can see this error even from a single threaded unit test.
If I can create a simple test, I will share it here.

@cowtowncoder
Copy link
Member

@hipatel7 Ok. The reason I asked about concurrency was since I was guessing this might be related to caching. It may still be -- there is one specific change to caching that is included in 2.8.0, but not in 2.7.x -- but fortunately it should be easier to reproduce.

@cowtowncoder
Copy link
Member

Fixed #1297, which while not necessarily related would be in sort of some area. It would be great to get a reproduction to fix the problem for 2.8.1.

@ghost
Copy link

ghost commented Jul 19, 2016

@cowtowncoder I am also facing this issue. I can not upgrade it to version 2.8.1. Can you suggest alternate way of parsing MAP to JSON.

@cowtowncoder
Copy link
Member

@jajoriaprerna 2.8.1 has not yet been released, but will be soon. The only work-around I can think of is to change type signature of the class that triggers the problem, but that may not be practical.

@cowtowncoder
Copy link
Member

Fixed via #1302

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants