Warn against, and handle null return from factory

1 files changed, 13 insertions, 1 deletions

diff --git a/yolean/src/main/java/com/yahoo/yolean/concurrent/Memoized.java b/yolean/src/main/java/com/yahoo/yolean/concurrent/Memoized.java
index 25bdd5b61c8..e8660504a8a 100644
--- a/yolean/src/main/java/com/yahoo/yolean/concurrent/Memoized.java
+++ b/yolean/src/main/java/com/yahoo/yolean/concurrent/Memoized.java
@@ -6,6 +6,9 @@ import static java.util.Objects.requireNonNull;
 
 /**
  * Wraps a lazily initialised resource which needs to be shut down.
+ * The wrapped supplier may not return {@code null}, and should be retryable on failure.
+ * If it throws, it will be retried if {@link #get} is retried. A supplier that fails to
+ * clean up partial state on failure may cause a resource leak.
  *
  * @author jonmv
  */
@@ -31,9 +34,15 @@ public class Memoized<T, E extends Exception> implements Supplier<T>, AutoClosea
 
     @Override
     public T get() {
+        // Double-checked locking: try the variable, and if not initialized, try to initialize it.
         if (wrapped == null) synchronized (monitor) {
-            if (factory != null) wrapped = factory.get();
+            // Ensure the factory is called only once, by clearing it once successfully called.
+            if (factory != null) wrapped = requireNonNull(factory.get());
             factory = null;
+
+            // If we found the factory, we won the initialization race, and return normally; otherwise
+            // if wrapped is non-null, we lost the race, wrapped was set by the winner, and we return; otherwise
+            // we tried to initialise because wrapped was cleared by closing this, and we fail.
             if (wrapped == null) throw new IllegalStateException("already closed");
         }
         return wrapped;
@@ -41,9 +50,12 @@ public class Memoized<T, E extends Exception> implements Supplier<T>, AutoClosea
 
     @Override
     public void close() throws E {
+        // Alter state only when synchronized with calls to get().
         synchronized (monitor) {
+            // Ensure we only try to close the generated resource once, by clearing it after picking it up here.
             T maybe = wrapped;
             wrapped = null;
+            // Clear the factory, to signal this has been closed.
             factory = null;
             if (maybe != null) closer.close(maybe);
         }