Class internal thread-safety

Question

I am writing a class in Java which is internally multithreaded, in that it initializes and uses a separate thread to update its private fields.

class Foo {
    private volatile Byte channel = new Byte(0);
    private volatile Byte mode = new Byte(0);

    public Foo() {
        Thread t = new Thread(new UpdateFields());
        t.setDaemon(true);
        t.start();
    }

    public Byte getChannel() {
        return this.channel;
    }  

    public Byte getMode() {
        return this.mode;
    }

    private class UpdateFields implements Runnable {
        @Override public void run() {
            Byte data[];
            //get new data[]...
            channel = data[0];
            mode = data[1];
        }
    } 
}

My question is, is this class internally thread-safe? From what I have read regarding immutable objects such as Byte etc. are that they are inherently thread-safe.

EDIT: added default values to fields

Answer 1

My question is, is this class internally thread-safe? From what I have read regarding immutable objects such as Byte etc. are that they are inherently thread-safe.

The thread-safety problem that I see with your class is that you are updating two fields that look to be related. It could be possible, because of race conditions, to see a new channel value and an old mode value. I would use a volatile ChannelMode object instead of your two volatile fields.

public class ChannelMode {
    private byte channel;
    private byte mode;
    public byte getChannel() {
       return channel;
    }
    public byte getMode() {
       return mode;
    }
}

Although not necessary, I like to use the Atomic* classes instead of volatile directly so I'd use an AtomicReference<ChannelMode>. So your code would look like:

private AtomicReference<ChannelMode> channelModeRef =
    new AtomicReference<ChannelMode>(
        new ChannelMode(INITIAL_CHANNEL, INITIAL_MODE));
...
Byte data[];
// get new data[]...
// atomic operation to set the new channel-mode
channelModeRef.set(new ChannelMode(data[0], data[1]);

If your inner thread updates the ChannelMode every so often then your class should be thread safe in that the values will be atomically updated and memory properly synchronized.

Answer 2

If you change the inner fields the class is not immutable. An immutable class doesn't change. This means that your class is not thread-safe.

That's true that if you do not expose any setters or any other method that changes your class it's supposed to immutable, but in this case you change it internally. The inherently thread-safe means that it cannot be changes so there you don't need to fear different threads would race and corrupt each other's data. This is certainly not the case here.