Search code examples
javasynchronizationlock-freepooling

Is this unsynchronized object pool implementation good/safe?


Can this object pool cause visibility problems with multiple threads? I'm specifically wondering about this kind of execution sequence:

  • Thread A - obtainObject()
  • Thread A - modifies object (lets say visibleState = 42)
  • Thread A - releaseObject()
  • Thread B - obtainObject() (get the object just released by A)
  • Thread A - does something unrelated or dies
  • Thread B - modifies object (lets say visibleState = 1)
  • Thread B - print objects visibleState
  • Thread B - releaseObject()

Could the modification from Thread A possibly become visible to Thread B after B has modified the state itself? (I know it doesn't occur in practice, but I can't figure out if and how the JLS/Javadoc guarantees this).

Here's the code, stripped down to only show the essentials. I left out generification and a factory to create objects.

import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;

public class ObjectPool {

/** Maximum number of objects to be kept in pool */
int maxPoolSize = 10;

/** First object in pool (LIFO queue) */
final AtomicReference<PoolableObject> firstObject = new AtomicReference<PoolableObject>();

/** How many objects are currently in the pool */
final AtomicInteger poolSize = new AtomicInteger();

/** Gets an object from the pool. If no object is available
 * from the pool, a new object is created and returned */
public PoolableObject obtainObject() {
    while (true) {
        PoolableObject object = firstObject.get();
        if (object == null)
            break;
        if (firstObject.compareAndSet(object, object.next)) {
            poolSize.decrementAndGet();
            return object;
        }
    }
    // no more objects in pool, create a new object
    return new PoolableObject();
}

/** Returns an object to the pool. */
public void releaseObject(final PoolableObject object) {
    while (true) {
        if (poolSize.get() >= maxPoolSize)
            break;
        final PoolableObject first = firstObject.get();
        object.next = first;
        if (firstObject.compareAndSet(first, object)) {
            poolSize.incrementAndGet();
            break;
        }
    }
}

}

The objects managed by the pool are supposed to inherit from this class:

public class PoolableObject {

/** Links objects in pool in single linked list. */
PoolableObject next;

public int visibleState;

}

Solution

  • As far as visibility goes, AtomicReference has the same memory barrier properties as a volatile variable.

    Among those properties are that all actions of a thread before a write to a volatile variable "happen-before" that write. Or, to put it another way, if source code shows an action happening before a volatile write, that action can't be re-ordered by the JITC to occur after the volatile write. Thus, the scenario cited in the question won't be a problem; changes to the object made before it is released will be visible to other threads that subsequently capture the object.

    However, I don't completely follow the way this pool is supposed to work, and there could be other problems. In particular, I'm not convinced that the lack of atomicity between firstObject and poolSize is safe. Threads can definitely see these variables in an inconsistent state, because they are not synchronized; I can't tell if that matters though.