Does this implementation work for a Lock-less Thread Safe Lazy Singleton in Java?

Question

This is what I have so far, am I going in the right direction? Aim is to use this in scenarios where one thread requires access to the singleton more frequently than other threads, hence lock-less code is desirable, I wanted to use atomic variables for practice.

public final class ThreadSafeLazyCompareAndSwapSingleton {

private ThreadSafeLazyCompareAndSwapSingleton(){}

private static volatile ThreadSafeLazyCompareAndSwapSingleton instance;
private final static AtomicBoolean atomicBoolean = new AtomicBoolean(false);

public static ThreadSafeLazyCompareAndSwapSingleton getCASInstance(){
    if (instance==null){
        boolean obs = instance==null;
        while (!atomicBoolean.compareAndSet(true, obs == (instance==null))){
            instance = new ThreadSafeLazyCompareAndSwapSingleton(); 
        }
    }
    return instance;
}

}

Leaving aside whether or not your implementation works, I'd recommend [the Initialization-on-demand holder idiom](http://en.wikipedia.org/wiki/Initialization-on-demand_holder_idiom) instead due to its simplicity and efficiency. — JAB, Jun 26 '14 at 18:39
Does this even compile, my guess is no. And in regards if this is working, no it isnt. The atomic is initially false and the only update expects true. — Durandal, Jun 26 '14 at 18:41
You have written `compareAndSet(/* expected */ true` how is the value going to be `true`? — Peter Lawrey, Jun 26 '14 at 18:46
@JohnVint I guess the first answers were too easy and reliable, the OP is looking for increasing complicated and unreliable ways of doing the same thing. ;) — Peter Lawrey, Jun 26 '14 at 18:47
@PeterLawrey That depends on the situation (is the singleton class a subclass? etc.), but yeah, you're probably correct in the general case. (Of course, you could always use composition instead of inheritance for that, but that's getting into extra stuff.) — JAB, Jun 26 '14 at 19:13
@JAB an enum can implement interfaces, but it can't have a parent class (actually it is `Enum`) nor can it have sub-classes. I try to avoid using Singletons and when I do I try to make them stateless. i.e. they only hold code. — Peter Lawrey, Jun 26 '14 at 19:21
@PeterLawrey I know, that's why I mentioned the Singleton-as-subclass situation. And even then, the subclass doesn't have to be stateful, it may just extend the functionality of its parent class (and in fact, the only reason I can think of to have a stateless singleton that isn't just a collection of static methods is to be able to customize the behavior of the singleton per usage via subclassing [which does imply obtaining the singleton through a factory of some sort in order to control what object is returned, if that's the way the design ends up going; I've mainly seen this used for tests]) — JAB, Jun 26 '14 at 19:30
@JAB If you use dependency injection, there is little reason to sub-class a singleton. I admit have I allowed singletons to be sub-classed in my libraries but I don't do this myself. Not everyone uses DI rigorously ;) — Peter Lawrey, Jun 26 '14 at 19:34
I wanted to concentrate discussion of a lock-less implementation using compare and swap. I should have made this more apparent in the question title. — newlogic, Jun 27 '14 at 08:52
@aranhakki You now have two answers satisfying everything you need for a lock-less solution. Mine in the previous question and yshavit below. What is it you are looking for? Yshavit even goes into great detail explaining how it's working. — John Vint, Jun 27 '14 at 13:34

score 0 · Answer 1 · answered Jun 26 '14 at 18:51

Uh, it is hard to read with those inline conditions/assigns. It is also pretty much non-standard idiom, so I wonder if you really want to use it.

It has at least the problem that it can create more than one instance (might be acceptable in your case, you cant really avoid that if you want to be lock-free). But I think it also might return more than one instance, which is not what you want to have I guess.

I am not sure if you need a boolean, you could also use a AtomicReferenceFieldUpdater directly on the instance field.

I think the obs is not needed, you would create and return a new instance if you can set the boolean, and loop otherwise:

if (instance!=null)
  return instance;

if (atomicBoolean.compareAndSet(false, true))
{
  instance = new ThreadSafeLazyCompareAndSwapSingleton();
  return instance;
}
while(instance==null);
return instance;

But I really dont think it is a good idea to go with this extra boolean.

OldCurmudgeon · Answer 2 · 2014-06-27T13:20:54.700

0

Something like this would be safer - but there are better ways than this.

public final class ThreadSafeLazyCompareAndSwapSingleton {

    // THE instance.
    private static volatile ThreadSafeLazyCompareAndSwapSingleton instance;
    // Catches just one thread.
    private final static AtomicBoolean trap = new AtomicBoolean(false);

    public static ThreadSafeLazyCompareAndSwapSingleton getInstance() {
        // All threads will spin on this loop until instance has been created.
        while (instance == null) {
            // ONE of the spinning threads will get past the trap. Probably the first one.
            if ( trap.compareAndSet(false, true)) {
                // By definition of CAS only one thread will get here and construct.
                instance = new ThreadSafeLazyCompareAndSwapSingleton();
            }
        }
        // By definition instance can never be null.
        return instance;
    }

    // Dont let anyone but me construct.
    private ThreadSafeLazyCompareAndSwapSingleton() {
    }

}

Note that this fails if an exception is thrown during the construction.

edited Jun 27 '14 at 13:20

answered Jun 26 '14 at 19:00

OldCurmudgeon

64,482
16
119
213

How does trap.compareAndSet(false, true) ever return true, am I missing something big here? – newlogic Jun 27 '14 at 09:11
@aranhakki - `compareAndSet` returns `true` if it successfully transitioned the field from it's old value to it's new value. In this case it set the atomic boolean to `true` because it was `false`. This ensures that only one thread gets into the construction section of the code. All other threads will just loop until that one has completed it's work. – OldCurmudgeon Jun 27 '14 at 09:53
Ah ok thanks, I have been interpreting as if the expected matches the update. So it works as the initial value is false, the first thread comes in and sets it to true. – newlogic Jun 27 '14 at 10:14
@aranhakki - Correct - I have added some comments to clarify. – OldCurmudgeon Jun 27 '14 at 13:22

yshavit · Answer 3 · 2014-06-26T19:05:57.213

A good mental approach here would be to separate threads into two categories: those that can instantiate the class, and those that can't. (For conciseness, I'll shorten the class name to just Singleton). Then you have to think about what each category of threads needs to do:

instantiating threads need to store the reference they create in instance and return it
all other threads need to wait until instance has been set, and then return it

Additionally, we need to ensure two things:

That there is a happens-before edge between the instantiation and all returns (including ones in non-instantiating threads). This is for thread safety.
That the set of instantiating threads has exactly one element (assuming either set is non-empty, of course). This is to ensure that there's only one instance.

Okay, so those are our four requirements. Now we can write code that satisfies them.

private final AtomicBoolean instantiated = new AtomicBoolean(false);
private static volatile Singleton instance = null;
// volatile ensures the happens-before edge

public static Singleton getInstance() {
    // first things first, let's find out which category this thread is in
    if (instantiated.compareAndSet(false, true) {
        // This is the instantiating thread; the CAS ensures only one thread
        // gets here. Create an instance, store it, and return it.
        Singleton localInstance = new Singleton();
        instance = localInstance;
        return localInstance;
    } else {
        // Non-instantiating thread; wait for there to be an instance, and
        // then return it.
        Singleton localInstance = instance;
        while (localInstance == null) {
            localInstance = instance;
        }
        return localInstance;
    }
}

Now, let's convince ourselves that each one of our conditions are met:

Instantiating thread creates an instance, stores it, and returns it: this is the "true" block of the CAS.
Other threads wait for an instance to be set, and then return it: That's what the while loop does.
There is a happens-before (HB) edge between instantiation and returning: For the instantiating thread, within-thread semantics ensure this. For all other threads, the volatile keyword ensures a HB edge between the write (in the instantiating thread) and the read (in this thread)
That the set of instantiating threads is exactly one large, assuming the method is ever invoked: The first thread to hit the CAS will have it return true; all others will return false.

So we're all set.

The general advice here is to break down your requirements into sub-requirements that are as specific as possible. Then you can address each one separately, which is easier to reason about.

Btw, as others have pointed out (in the comments as well as [in your other question](http://stackoverflow.com/questions/24434566)), there are simpler ways to get lock-free singletons in Java. You really, really should use those if this is for real code. My answer is assuming that this is more of a mental exercise to learn multithreading. — yshavit, Jun 26 '14 at 19:14

Does this implementation work for a Lock-less Thread Safe Lazy Singleton in Java?

3 Answers3