Commit 2a60f733 authored by Rajiv Prabhakar's avatar Rajiv Prabhakar
Browse files

v2.0.5 release: bug fix for DynamicConstant. Ensure defaultSupplier is only...

v2.0.5 release: bug fix for DynamicConstant. Ensure defaultSupplier is only called once, even for multiple parallel get() calls
parent 4915e756
Pipeline #25364038 passed with stage
in 1 minute and 51 seconds
......@@ -6,7 +6,7 @@
<groupId>com.rajivprab</groupId>
<artifactId>cava</artifactId>
<version>2.0.4</version>
<version>2.0.5</version>
<name>Cava: Clean Java</name>
<description>A library that enables users to write minimal, clean and simple Java</description>
......
......@@ -27,6 +27,7 @@ import java.util.function.Supplier;
public class DynamicConstant<T> {
private final AtomicReference<T> value = new AtomicReference<>();
private final Supplier<T> defaultSupplier;
private T defaultSupplied;
public static <T> DynamicConstant<T> noDefault() {
return withDefault(null);
......@@ -46,21 +47,34 @@ public class DynamicConstant<T> {
}
public T get() {
T data = this.value.get();
if (data == null) {
T def = Validatec.notNull(defaultSupplier.get(), "Default/set value does not exist, or is null");
return set(def);
}
return data;
T value = this.value.get();
return value == null ? setDefault() : value;
}
// If set is called with data that is incompatible with the previous get/set calls,
// An exception will be thrown, and no data will be changed.
public T set(T newVal) {
Validatec.notNull(newVal, "Cannot set value to null");
// TODO Enhancement: Is there any danger that because set/get are not synchronized,
// the newVal memory reference can be written into value, and visible to other threads,
// even before the newVal object has completed its construction? Ie, the same reason why Java DCL fails?
// Because value is a AtomicReference, this should not be a problem?
// https://stackoverflow.com/questions/3964211/when-to-use-atomicreference-in-java
boolean updated = value.compareAndSet(null, newVal);
Validate.isTrue(updated || value.get().equals(newVal),
"New value: %s, does not match existing value: %s", newVal, value.get());
return newVal;
}
private synchronized T setDefault() {
if (defaultSupplied == null) {
defaultSupplied = Validatec.notNull(defaultSupplier.get(), "Default/set value has not been given");
return set(defaultSupplied);
} else {
return defaultSupplied;
}
}
}
\ No newline at end of file
package org.rajivprab.cava;
import com.google.common.collect.ImmutableList;
import org.junit.Assert;
import org.junit.Test;
import org.rajivprab.cava.exception.CheckedExceptionWrapper;
import java.util.Random;
import java.util.Set;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import static com.google.common.truth.Truth.assertThat;
......@@ -95,7 +104,7 @@ public class DynamicConstantTest extends TestBase {
dynamicConstant.get();
Assert.fail();
} catch (IllegalArgumentException e) {
assertThat(e).hasMessageThat().isEqualTo("Default/set value does not exist, or is null");
assertThat(e).hasMessageThat().isEqualTo("Default/set value has not been given");
}
}
......@@ -106,7 +115,7 @@ public class DynamicConstantTest extends TestBase {
dynamicConstant.get();
Assert.fail();
} catch (IllegalArgumentException e) {
assertThat(e).hasMessageThat().isEqualTo("Default/set value does not exist, or is null");
assertThat(e).hasMessageThat().isEqualTo("Default/set value has not been given");
}
}
......@@ -120,6 +129,31 @@ public class DynamicConstantTest extends TestBase {
}
}
@Test
public void supplierReturnsDifferentObjectEachTime_multipleParallelGets_shouldOnlyUseSupplierOnce() {
RandomInt.numCalls.set(0);
DynamicConstant<Integer> constant = DynamicConstant.lazyDefault(RandomInt::get);
int numThreads = 100;
CountDownLatch latch = new CountDownLatch(numThreads);
Set<Integer> values = IntStream.range(0, numThreads)
.mapToObj(i -> ThreadUtilc.fork(() -> getAfterLatchCountdown(constant, latch)))
.collect(ImmutableList.toImmutableList()).stream()
.map(ThreadUtilc::get)
.collect(Collectors.toSet());
assertThat(values).hasSize(1);
assertThat(RandomInt.numCalls.get()).isEqualTo(1);
}
private static <T> T getAfterLatchCountdown(DynamicConstant<T> constant, CountDownLatch latch) {
try {
latch.countDown();
latch.await();
return constant.get();
} catch (Exception e) {
throw new CheckedExceptionWrapper(e);
}
}
private static class LandmineHolder {
static final String VAL = getVal();
......@@ -135,4 +169,15 @@ public class DynamicConstantTest extends TestBase {
return "Holder Value";
}
}
private static class RandomInt {
private static final Random RNG = new Random();
private static final AtomicInteger numCalls = new AtomicInteger(0);
public static int get() {
ThreadUtilc.fork(numCalls::incrementAndGet);
return RNG.nextInt();
}
}
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment