NullPointerException in LazyDeserializer due to error in double-checked locking code

[henricm]

The unwrap() method may return a null deserialiser if two threads access the method simultaneously.

elasticsearch-java/java-client/src/main/java/co/elastic/clients/json/LazyDeserializer.java

Lines 41 to 53 in 3e6f05c

	protected JsonpDeserializer<T> unwrap() {
	// See SEI CERT LCK10-J https://wiki.sei.cmu.edu/confluence/x/6zdGBQ
	JsonpDeserializer<T> d = deserializer;
	if (d == null) {
	synchronized (this) {
	if (deserializer == null) {
	d = ctor.get();
	deserializer = d;
	}
	}
	}
	return d;
	}

Stacktrace:

Caused by: java.lang.NullPointerException: null
	at co.elastic.clients.json.DelegatingDeserializer$SameType.deserialize(DelegatingDeserializer.java:43) ~[elasticsearch-java-7.16.0.jar:na]
	at co.elastic.clients.json.ObjectDeserializer$FieldObjectDeserializer.deserialize(ObjectDeserializer.java:72) ~[elasticsearch-java-7.16.0.jar:na]
	at co.elastic.clients.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:176) ~[elasticsearch-java-7.16.0.jar:na]
	at co.elastic.clients.json.ObjectDeserializer.deserialize(ObjectDeserializer.java:137) ~[elasticsearch-java-7.16.0.jar:na]
	at co.elastic.clients.json.JsonpDeserializer.deserialize(JsonpDeserializer.java:75) ~[elasticsearch-java-7.16.0.jar:na]
	at co.elastic.clients.json.ObjectBuilderDeserializer.deserialize(ObjectBuilderDeserializer.java:79) ~[elasticsearch-java-7.16.0.jar:na]
	at co.elastic.clients.json.DelegatingDeserializer$SameType.deserialize(DelegatingDeserializer.java:43) ~[elasticsearch-java-7.16.0.jar:na]
	at co.elastic.clients.transport.endpoints.EndpointWithResponseMapperAttr$1.deserialize(EndpointWithResponseMapperAttr.java:56) ~[elasticsearch-java-7.16.0.jar:na]
	at co.elastic.clients.transport.rest_client.RestClientTransport.decodeResponse(RestClientTransport.java:325) ~[elasticsearch-java-7.16.0.jar:na]
	at co.elastic.clients.transport.rest_client.RestClientTransport.getHighLevelResponse(RestClientTransport.java:291) ~[elasticsearch-java-7.16.0.jar:na]
	at co.elastic.clients.transport.rest_client.RestClientTransport.performRequest(RestClientTransport.java:144) ~[elasticsearch-java-7.16.0.jar:na]
	at co.elastic.clients.elasticsearch.ElasticsearchClient.search(ElasticsearchClient.java:1487) ~[elasticsearch-java-7.16.0.jar:na]

Debugging screenshot where you can see deserializer is set, but d is null:

Looking at the article linked to in the source code, it looks like the "Compliant Solution (Immutable)" is desired. In this case, there is one assignment (d = deserializer) missing just before the null check inside the synchonized block.

So the code below should work better, making sure d is getting assigned with deserializer, for the cases where it has been assigned by another thread after the current thread has entered the synchronized block:

    protected JsonpDeserializer<T> unwrap() {
        // See SEI CERT LCK10-J https://wiki.sei.cmu.edu/confluence/x/6zdGBQ
        JsonpDeserializer<T> d = deserializer;
        if (d == null) {
            synchronized (this) {
                d = deserializer;
                if (d == null) {
                    d = ctor.get();
                    deserializer = d;
                }
            }
        }
        return d;
    }

Version: 7.16.0

[swallez]

Thanks for the report @henricm. This has been fixed in #59

[henricm]

Awesome! Thanks for the quick fix. 👍