java builder pattern concurrent changes

Question

I have an immutable class "Immutable" with a nested builder "Immutable.Builder" to avoid concurrency issues.

Now i have an instance "randomInstance" of a random class "RandomClass" with a field "Immutable immutable = Immutable.Builder.instanceOf()".

Now i have two concurrent threads who instantiate Immutable.Builder instances off of the Immutable instance in that field, make changes to the builder, call build() to get a new Immutable instance, and overwrite the instance of that field with the new Immutable instance. The classic use case for the builder pattern, no?

When both threads instantiate the builder off of the same instance of field before one of them writes the new instance back into the field, the "changes" made by the first thread that writes its new instance in the field will be subject to the garbage collector and essentially lost, as soon as the second thread writes its new instance into the field, is it not?

What approaches are there, to make sure, all "changes" are taken into account?

EDIT I didn't this this question would benefit from a working example but apparently I was wrong, so here it is as slim as i could do it:

public class Main {

    public static void main(String[] args) {

        Immutable immutable = new Immutable.Builder(0).build();
        RandomClass randomClass = new RandomClass();

        java.lang.Thread t1 = new java.lang.Thread(new Thread(randomClass, true));
        t1.start();
        java.lang.Thread.sleep(500);
        java.lang.Thread t2 = new java.lang.Thread(new Thread(randomClass, false));
        t2.start();

    }
}

public class RandomClass {
    public Immutable immutable = new Immutable.Builder(0).build();
}

public class Immutable {

    private int field;

    private Immutable(Builder builder) {
        this.field = builder.field;
    }

    public static class Builder {

        private int field;

        public Builder(int arg) {
            this.field = arg;
        }

        public Builder setField(int arg) {
            this.field = arg;

            return this;
        }

        public Immutable build() {
            return new Immutable(this);
        }

    }

    public Builder builder() {
        return new Builder(this.field);
    }

}

import java.util.Random;

public class Thread implements Runnable {

    private RandomClass randomClass;

    public Thread(RandomClass randomClass) {
        this.randomClass = randomClass;
    }

    @Override
    public void run() {
        while(true) {
            int i = new Random().nextInt();
            this.randomClass.immutable = this.randomClass.immutable.builder().setField(i).build();
        }
    }

}

Answer 1

The problem as I understand it is basically a typical lost update. One thread requests a builder object to create a new instance based off the existing Immutable value, applies its changes and sets it on RandomObject. Meanwhile another thread does the same. One of them sets their changes last and overwrites the changes of the other thread.

Source of the problem is of course improper synchronization on the immutable field of the RandomClass class (which is, ironically, mutable). We'll need to make updates properly synchronized, and as we want to support 'check-then-act' updates (i.e. updates that act upon the previous value) we'll need to support atomic updating.

I'll give 2 possible solutions :

1. Instead of a setter, provide a method that accepts a mutator, and synchronize access to the field.

   For this I would change RandomClass to :

   public class RandomClass {
       private final Object monitor = new Object();
       @GuardedBy("monitor")
       private Immutable immutable = new Immutable.Builder(0).build();
   
       public Immutable getImmutable() {
           synchronized (monitor) {
               return immutable;
           }
       }
   
       public void updateImmutable(Function<Immutable, Immutable> mutator) {
           synchronized (monitor) {
               immutable = mutator.apply(immutable);
           }
       }
   }
   

   Clients would update like this :

   randomObject.updateImmutable(oldValue -> oldValue.builder().setField(oldValue.getField() * 2));
   

   This approach leverages use of Java 8 lambdas, but is otherwise a very classic approach, using a synchronized block. It will do well performance wise even under heavier contention. On the downside, reads also pay a synchronization cost.

2. Store the reference to Immutable in an AtomicReference, the setter needs to supply the old value along with the new one, returns a boolean to indicate success.

   For this I would change RandomClass to :

   public class RandomClass {
       private AtomicReference<Immutable> immutable = new AtomicReference(new Immutable.Builder(0).build());
   
       public Immutable getImmutable() {
           return immutable.get();
       }
   
       public boolean updateImmutable(Immutable oldValue, Immutable newValue) {
           return immutable.compareAndSet(oldValue, newValue);
       }
   }
   

   Clients would update like this :

   boolean updateSuccess = false;
   while (!updateSuccess) {
       Immutable oldValue = randomObject.getImmutable();
       Immutable newValue = oldValue.builder().setField(oldValue.getField() * 2);
       updateSuccess = randomObject.updateImmutable(oldValue, newValue);
   }
   

   This is a lockless approach, but at the price of more burden and verbosity for the client. It will perform great under low contention, but will suffer under heavy contention. Reads are unburdened by synchronization.

3. Combine 1. and 2.

   Use the setter of solution 1. but instead of synchronizing on a monitor, you use the AtomicReference approach of 2. internally.

   This can get the best of both worlds, the elegance of use for the client, leveraging lambdas. Lockless approach, at little to no cost on reads, but not great under heavy contention.