Usage of AtomicBoolean

I would like to find out if following usage of an AtomicBoolean as a flag variable is correct and good for a multi-threaded application. It is expected that data is going to be loaded only once and when the application is at very low load. It is also expected that the list of SomeObjects is going to be read quite frequently by multiple threads at peak load, probably thousands of time in an hour so that is why I want to avoid hitting the database. The list is not going to be very large so keeping it in memory is not going to be a problem.

public class HibernateDao extends SomeOtherClass implements Dao {   

    //Need suggestions for correct usage of following variable
    private AtomicBoolean dataLoaded = new AtomicBoolean();
    private List someObjects;

    @Override
    public List getAllSomeObjects() {       
        if (dataLoaded.getAndSet(false)) {
            //aim is to atomically load the list only once and avoid calling loadAll 
            //every time list is used           
            someObjects= (List) getHibernateTemplate().loadAll(SomeObject.class);           
        }
        return someObjects;
    }

    @Override
    public void updateAllSomeObjects(Collection someObjects) {
            //replaceAll is part of parent class and it removed and reloads
            //data for the entity i.e. SomeObject
        replaceAll(SomeObject.class, someObjects);
            //set this flag atomically so that data is fetched from DB next time
        dataLoaded.getAndSet(true);
    }

}

You are attempting to use the AtomicBoolean to satisfy two requirements:

-
The first thread to access the data loads it while all others wait until it's been loaded the first time.

-
Another thread can update the data for future callers.

Neither of these is working perfectly.

-
While only one thread will load the data, all other threads will receive null without waiting until that first thread completes the loading process.

-
Because the list of objects is updated after the memory barrier, other threads are not guaranteed to see the new reference immediately.

Solving the first problem is simple enough: use a CountDownLatch to cause all incoming threads to block until the first list of objects is set. Ideally the system would start another thread to specifically load the data rather than letting the first random thread to come along and do it. This isn't necessary (see below), but it's much cleaner.

For the second problem, wrap the list of objects in an AtomicReference to supply a correct memory barrier. You might be thinking, "Oh no! Not two synchronizers for each access!" but Java's synchronization primitives have improved greatly over the years, and the atomic value holders are even cheaper than full synchronization.

public class HibernateDao extends SomeOtherClass implements Dao {
    private CountDownLatch dataLoaded = new CountDownLatch();
    private AtomicReference> data = new AtomicReference<>();

    @Override
    public List getAllSomeObjects() {
        dataLoaded.await();          // wait until first countDown call
        return data.get();
    }

    @Override
    public void updateAllSomeObjects(Collection someObjects) {
        replaceAll(SomeObject.class, someObjects);
        data.set(someObjects);
        dataLoaded.countDown();      // release any waiting threads
    }
}

If you really must have the first thread perform the initial loading rather than a separate thread that calls updateAllSomeObjects on its own, you can add back your beloved AtomicBoolean. This would be my last choice, however.


Note that while you can use if (!ab.getAndSet(true)) it's clearer to use compareAndSet as it indicates that you're performing a tested set rather than allowing any thread to set the new value.

private AtomicBoolean firstCaller = new AtomicBoolean();

    @Override
    public List getAllSomeObjects() {
        if (firstCaller.compareAndSet(false, true)) {
            updateAllSomeObjects((List) getHibernateTemplate().loadAll(SomeObject.class));
        }
        else {
            dataLoaded.await();      // wait until first countDown call
        }
        return data.get();
    }

StackExchange Code Review Q#49514, answer score: 6