Warn against, and handle null return from factory
@@ -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);
}
