Thread safety of a fluent like class using clone() and non final fields

Question

This fluent like class is not strictly immutable because the fields are not final, but is it thread safe, and why?

The thread safety issue I'm concerned with is not the race condition, but the visibility of the variables. I know there is a workaround using final variables and a constructor instead of clone() + assignment. I just want to know if this example is a viable alternative.

public class IsItSafe implements Cloneable {

    private int foo;
    private int bar;

    public IsItSafe foo(int foo) {
        IsItSafe clone = clone();
        clone.foo = foo;
        return clone;
    }

    public IsItSafe bar(int bar) {
        IsItSafe clone = clone();
        clone.bar = bar;
        return clone;
    }

    public int getFoo() {
        return foo;
    }

    public int getBar() {
        return bar;
    }

    protected IsItSafe clone() {
        try {
            return (IsItSafe) super.clone();
        } catch (CloneNotSupportedException e) {
            throw new Error(e);
        }
    }
}

Answer 1

You are not holding a Lock while setting the field and as you mention yourself the field is not final.

Therefore from a visibility standpoint this approach is not thread-safe.

Some further clarifications here: https://stackoverflow.com/a/9633968/136247

Update regarding the question of using volatile:

For the sake of the argument using volatile fixes the threading issue here.
However you should reconsider final fields and a copy constructor because:

• Field access will be slightly faster (the reads can always come from the cpu cache)
• You avoid the discouraged use of clone (see Effective Java by Josh Bloch)
• Constructor with final fields is a known idiom for immutable classes and will be easily recognised by readers of the code
• Marking fields volatile while intending for them to be immutable is a contradiction in and of itself ;)