From 611e8b1519fb409edac887a7c2a7030b996283ae Mon Sep 17 00:00:00 2001 From: Sylvain Wallez Date: Tue, 14 Dec 2021 14:24:42 +0100 Subject: [PATCH] Fix race condition in LazyDeserializer initialization --- .../clients/json/LazyDeserializer.java | 9 +-- .../clients/json/LazyDeserializerTest.java | 69 +++++++++++++++++++ 2 files changed, 74 insertions(+), 4 deletions(-) create mode 100644 java-client/src/test/java/co/elastic/clients/json/LazyDeserializerTest.java diff --git a/java-client/src/main/java/co/elastic/clients/json/LazyDeserializer.java b/java-client/src/main/java/co/elastic/clients/json/LazyDeserializer.java index c70cc167f..61a770327 100644 --- a/java-client/src/main/java/co/elastic/clients/json/LazyDeserializer.java +++ b/java-client/src/main/java/co/elastic/clients/json/LazyDeserializer.java @@ -41,14 +41,15 @@ class LazyDeserializer extends DelegatingDeserializer.SameType { protected JsonpDeserializer unwrap() { // See SEI CERT LCK10-J https://wiki.sei.cmu.edu/confluence/x/6zdGBQ JsonpDeserializer d = deserializer; - if (d == null) { + if (d != null) { + return d; + } else { synchronized (this) { if (deserializer == null) { - d = ctor.get(); - deserializer = d; + deserializer = ctor.get(); } } + return deserializer; } - return d; } } diff --git a/java-client/src/test/java/co/elastic/clients/json/LazyDeserializerTest.java b/java-client/src/test/java/co/elastic/clients/json/LazyDeserializerTest.java new file mode 100644 index 000000000..9c2ac4075 --- /dev/null +++ b/java-client/src/test/java/co/elastic/clients/json/LazyDeserializerTest.java @@ -0,0 +1,69 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package co.elastic.clients.json; + +import org.junit.Assert; +import org.junit.Test; + +import java.util.concurrent.CompletableFuture; + +public class LazyDeserializerTest extends Assert { + + + @Test + public void testConcurrentInit() throws Exception { + // See https://github.com/elastic/elasticsearch-java/issues/58 + + final LazyDeserializer ld = new LazyDeserializer<>(JsonpDeserializer::stringDeserializer); + + CompletableFuture> fut1; + CompletableFuture> fut2; + + // Lock the mutex and start 2 threads that will compete for it. + synchronized (ld) { + fut1 = futureUnwrap(ld); + fut2 = futureUnwrap(ld); + } + + // We should see the same non-null results everywhere + assertNotNull(fut1.get()); + assertNotNull(fut2.get()); + + final JsonpDeserializer unwrapped = ld.unwrap(); + assertEquals(unwrapped, fut1.get()); + assertEquals(unwrapped, fut2.get()); + + } + + private CompletableFuture> futureUnwrap(LazyDeserializer d) { + + final CompletableFuture> result = new CompletableFuture<>(); + + new Thread(() -> { + try { + result.complete(d.unwrap()); + } catch (Throwable e) { + result.completeExceptionally(e); + } + }).start(); + + return result; + } +}