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

Fix issue 172 #173

Merged
merged 5 commits into from
Jul 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ protected JsonpDeserializer<T> unwrap() {
JsonpDeserializer<T> d = deserializer;
if (d == null) {
synchronized (this) {
if (deserializer == null) {
d = deserializer;
if (d == null) {
d = ctor.get();
deserializer = d;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

package org.opensearch.client.opensearch.json;

import org.opensearch.client.json.JsonpDeserializer;
import org.opensearch.client.json.JsonpMapper;
import org.opensearch.client.json.jackson.JacksonJsonpMapper;
import org.opensearch.client.json.jsonb.JsonbJsonpMapper;
Expand All @@ -55,6 +56,8 @@
import java.io.Writer;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicInteger;

public class JsonpMapperTest extends Assert {

Expand Down Expand Up @@ -116,6 +119,60 @@ public void testJacksonCustomJsonFactory() {
IOUtils.closeWhileHandlingException(writer);
}

@Test
public void testConcurrentLazyResolve() throws Exception {
// Test fix for issue 172 - concurrency error in LazyDeserializer
// This latch holds off resolution of the LazyDeserializer until we're sure that
// two threads are attempting it simultaneously
CountDownLatch trigger = new CountDownLatch(1);
JsonpDeserializer<Integer> deserializer = JsonpDeserializer.lazy(() -> {
try {
trigger.await();
} catch (Exception e) {
throw new RuntimeException("Interrupted", e);
}
return JsonpDeserializer.integerDeserializer();
});

// Two threads will attempt to deserialize. They should both be successful
final AtomicInteger successes = new AtomicInteger(0);
final JsonpMapper mapper = new JsonbJsonpMapper();
Runnable threadProc = () -> {
JsonParser parser = mapper.jsonProvider().createParser(new StringReader("0"));
try {
// Prior to fix, one of these would throw NPE because its
// LazyDeserializer resolution would return null
deserializer.deserialize(parser,mapper);
successes.incrementAndGet();
} catch (Throwable e) {
// We'll notice that we failed to increment successes
}
};
// Two identical threads
Thread thread1 = new Thread(threadProc);
thread1.setDaemon(true);
thread1.start();
Thread thread2 = new Thread(threadProc);
thread2.setDaemon(true);
thread2.start();

// Wait until both threads are blocked waiting LazyDeserializer resolution
do {
try {
Thread.sleep(5);
mtimmerm marked this conversation as resolved.
Show resolved Hide resolved
} catch (Exception e) {
break;
}
} while (thread1.getState() == Thread.State.RUNNABLE || thread2.getState() == Thread.State.RUNNABLE);

// Now allow resolution to proceed and wait for results
trigger.countDown();
thread1.join();
thread2.join();
assertEquals(2, successes.get());
}


private void testSerialize(JsonpMapper mapper, String expected) {

SomeClass something = new SomeClass();
Expand Down