javamultithreadingsynchronizationnonblockingatomicreference

Convert synchronized methods to non-blocking algorithm


Just find some information about non-blocking algorithms, so want to use them in practice. I changed some code from synchronized to non-blocking, so I want to ask does I made everything right and saved previous functionality.

synchronized code:

protected PersistentState persistentState;
protected ClassConstructor(final ID id)
{
    super(id);
    this.persistentState = PersistentState.UNKNOWN;
}
public final synchronized PersistentState getPersistentState()
{
    return this.persistentState;
}

protected synchronized void setPersistentState(final PersistentState newPersistentState)
{
    if (this.persistentState != newPersistentState)
    {
        this.persistentState = newPersistentState;
        notifyPersistentStateChanged();
    }
}

my alternative in non-blocking algorithm:

     protected AtomicReference<PersistentState> persistentState;
  protected ClassConstructor(final ID id)
    {
        super(id);
        this.persistentState = new AtomicReference<PersistentState>(PersistentState.UNKNOWN);
    }
   public final PersistentState getPersistentState()
    {
        return this.persistentState.get();
    }

    protected void setPersistentState(final PersistentState newPersistentState)
    {
        PersistentState tmpPersistentState;
        do
        {
            tmpPersistentState = this.persistentState.get();
        }
        while (!this.persistentState.compareAndSet(tmpPersistentState, newPersistentState));
        // this.persistentState.set(newPersistentState); removed as not necessary 
        notifyPersistentStateChanged();
    }

Do I've done everything correctly, or I missed something? Any suggestions for the code and using non-blocking method for setting abject in general?


Solution

  • Depends what you mean by thread-safe. What do you want to happen if two threads try to write at the same time? Should one of them chosen at random be chosen as the correct new value?

    This would be it at it's simplest.

    protected AtomicReference<PersistentState> persistentState = new AtomicReference<PersistentState>(PersistentState.UNKNOWN);
    
    public final PersistentState getPersistentState() {
        return this.persistentState.get();
    }
    
    protected void setPersistentState(final PersistentState newPersistentState) {
        persistentState.set(newPersistentState);
        notifyPersistentStateChanged();
    }
    
    private void notifyPersistentStateChanged() {
    }
    

    This would still call notifyPersistentStateChanged in all cases, even if the state hasn't changed. You need to decide what should happen in that scenario (one thread makes A -> B and another goes B -> A).

    If, however, you need to only call the notify if successfully transitioned the value you could try something like this:

     protected void setPersistentState(final PersistentState newPersistentState) {
        boolean changed = false;
        for (PersistentState oldState = getPersistentState();
                // Keep going if different
                changed = !oldState.equals(newPersistentState)
                // Transition old -> new successful?
                && !persistentState.compareAndSet(oldState, newPersistentState);
                // What is it now!
                oldState = getPersistentState()) {
            // Didn't transition - go around again.
        }
        if (changed) {
            // Notify the change.
            notifyPersistentStateChanged();
        }
    }