Fix racy read of null weak/soft old value when writing a new value (f…

…ixes #568)
ben-manes · Jun 29, 2021 · afec608 · afec608
1 parent f279485
commit afec608
Showing 1 changed file with 28 additions and 2 deletions.
diff --git a/caffeine/src/javaPoet/java/com/github/benmanes/caffeine/cache/node/AddValue.java b/caffeine/src/javaPoet/java/com/github/benmanes/caffeine/cache/node/AddValue.java
@@ -24,6 +24,7 @@
 import javax.lang.model.element.Modifier;
 
 import com.squareup.javapoet.ClassName;
+import com.squareup.javapoet.CodeBlock;
 import com.squareup.javapoet.FieldSpec;
 import com.squareup.javapoet.MethodSpec;
 
@@ -44,7 +45,7 @@ protected boolean applies() {
   protected void execute() {
     context.nodeSubtype
         .addField(newValueField())
-        .addMethod(newGetter(valueStrength(), vTypeVar, "value", Visibility.PLAIN))
+        .addMethod(makeGetValue())
         .addMethod(newGetRef("value"))
         .addMethod(makeSetValue())
         .addMethod(makeContainsValue());
@@ -60,6 +61,30 @@ private FieldSpec newValueField() {
     return fieldSpec.build();
   }
 
+  /** Creates the getValue method. */
+  private MethodSpec makeGetValue() {
+    MethodSpec.Builder getter = MethodSpec.methodBuilder("getValue")
+        .addModifiers(context.publicFinalModifiers())
+        .returns(vTypeVar);
+    if (valueStrength() == Strength.STRONG) {
+      getter.addStatement("return ($T) $L.get(this)", vTypeVar, varHandleName("value"));
+      return getter.build();
+    }
+
+    CodeBlock code = CodeBlock.builder()
+        .beginControlFlow("for (;;)")
+            .addStatement("$1T<V> ref = ($1T<V>) $2L.get(this)",
+                Reference.class, varHandleName("value"))
+            .addStatement("V value = ref.get()")
+            .beginControlFlow(
+                "if ((value != null) || (ref == $L.getVolatile(this)))", varHandleName("value"))
+                .addStatement("return value")
+            .endControlFlow()
+        .endControlFlow()
+        .build();
+    return getter.addCode(code).build();
+  }
+
   /** Creates the setValue method. */
   private MethodSpec makeSetValue() {
     MethodSpec.Builder setter = MethodSpec.methodBuilder("setValue")
@@ -70,9 +95,10 @@ private MethodSpec makeSetValue() {
     if (isStrongValues()) {
       setter.addStatement("$L.set(this, $N)", varHandleName("value"), "value");
     } else {
-      setter.addStatement("(($T<V>) getValueReference()).clear()", Reference.class);
+      setter.addStatement("$1T<V> ref = ($1T<V>) getValueReference()", Reference.class);
       setter.addStatement("$L.set(this, new $T($L, $N, referenceQueue))",
           varHandleName("value"), valueReferenceType(), "getKeyReference()", "value");
+      setter.addStatement("ref.clear()");
     }
 
     return setter.build();