Is this java functional version of the double checked lock safe?

Question

I'm trying to implement a reusable functional-like version of the double checked lock (DCL) pattern in Java.

Indeed, there are many known problems with the DCL pattern in Java, like those ones. So I'm trying to check if the solution I develop has any flaws.

Here is the DCL executor code:

public class DoubleCheckedLockExecutor {

    public <T> T getOrCreate(Supplier<T> supplier, Supplier<T> builder, Consumer<T> consumer, Predicate<T> build) {

        if (build.test(supplier.get())) {
            synchronized (this) {
                if (build.test(supplier.get())) {
                    consumer.accept(builder.get());
                }
            }
        }
        return supplier.get();
    }
}

And here a Singleton class that uses it:

public class Singleton {
    private static Singleton instance = null;
    private static final AtomicInteger instanceCount = new AtomicInteger();
    private static final DoubleCheckedLockExecutor dclExec = new DoubleCheckedLockExecutor();

    private Singleton() {
        instanceCount.incrementAndGet();
    }

    public static Singleton getInstance() {
        return dclExec.getOrCreate(() -> instance, Singleton::new, s -> instance = s, s -> s == null);
    }

    public static int getInstanceCount() {
        return instanceCount.get();
    }
}

And finally some test code:

@Test
public final void testGetOrCreate() {
    int calls = 1000;
    ExecutorService executor = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors());
    try {
        for (int i = 0; i < calls; i++) {
            executor.execute(() -> Singleton.getInstance());
        }
    } catch (Exception e) {
        e.printStackTrace();
    }
    executor.shutdown();
    while (!executor.isTerminated()) {
    }
    assertEquals(1, Singleton.getInstanceCount());
}

All tests and analysis I did showed no problems (duplicated instances, for example). But multi-thread and concurrency test is not such easy task for me. So could you guys help me out with this? Could I say this implementation is thread safe and produce the expected results?

I want to say no. I believe this falls prey to the same issues as the normal DCL solutions where the static field `instance` should be volatile to alleviate data-race issues. See [this answer](https://stackoverflow.com/questions/49840773/need-of-volatile-keyword-in-case-of-dcl) for the standard `volatile` approach. What I'm unsure of is if the use of the supplier would return a fully initialized object, or fall prey to the same issues as the non-wrapped constructor call (i.e because of use of method-reference). — flakes, Jul 04 '19 at 04:26
I think it depends entirely upon what your supplier and predicate looks like. In your example, it doesn't seem like there is any happens-before relation between the reads and writes to `instance` that would prevent a data race. Although, I wonder if you change your code such that your predicate checks `instanceCount` and if you make sure to initialize all your fields in the singleton before calling `instanceCount.incrementAndGet()` if that would be enough. — Eric, Jul 04 '19 at 08:53

Is this java functional version of the double checked lock safe?

0 Answers0