Should getters/setters of primitive types be locked with ReadWriteLock in a multithreading application?

Question

I have a Java class that is used in a multithreading application. Concurrent access is very likely. Multiple concurrent read operations should not block so I'm using a ReadWrite lock.

class Example {
    private final ReadWriteLock lock = new ReentrantReadWriteLock();
    private int i;
    private boolean b;

    public int getI() {
      lock.readLock().lock();
      final int tmp = i;
      lock.readLock().unlock(),
      return tmp;
    }

    public void setI( final int i ) {
      lock.writeLock().lock();
      this.i = i;
      lock.writeLock().unlock();
    }

    public boolean getB() {
      lock.readLock().lock();
      final boolean tmp = b;
      lock.readLock().unlock(),
      return tmp;
    }

    public void setB( final boolean b ) {
      lock.writeLock().lock();
      this.b = b;
      lock.writeLock().unlock();
    }
}

For simplicity I left out the try...finally blocks around the locks in this example.

I'm wondering if it's necessary (or let's say recommended) to lock/synchronize getters and setters of primitive types? I know that assign and return operations in Java are atomic. However by using these locks don't I make sure that every accessor gets the latest value (equals to using volatile)?

What if the primitives were double or long?

Answer 1

It depends.

Note, however, that usually you need to synchronize operations at more coarse-grained level, for example, this:

Example e = ...;

synchronized (e) {
    e.setI(e.getI() + 10);
}

For such scenarios your internal locks are redundant. Therefore perhaps it would be better to apply external synchronization where you use these objects, rather than internal synchronization.

Answer 2

Ask yourself if this class can be implemented as an immutable class. It will be easier to work with and will be inherently thread-safe. You will not have to worry about concurrency, locks, synchronization etc.

Example of an immutable class:

final class Example {
    private final int i;
    private final boolean b;

    public Example(int i, boolean b){
        this.i = i ;
        this.b = b;
    }

    public int getI() {
        return i;
    }

    public boolean getB() {
        return b;
    }
}

Answer 3

You should not use lock to primitive types, String (they are immutable) and thread-safe types (like collections from "concurrent" package).

Answer 4

I would design your application so that you will not have concurrent access to a raw data type like this. Adding such low level locking is likely to slow down your application so much it is not worth multi-threading your application.

e.g. Say you have a 32 core system which scales perfectly and runs 32x faster than on 1 core. However a field access without locking takes 1 ns, with locking takes 1 us (1000 ns) so in the end your application could take ~30x slower. (1000 slower / 32 faster) If you have only 4 cores, it could be hundreds of times slower, defeating the purpose of having multi-threads in the first place. IMHO.

Answer 5

you have something like AtomicInteger in java, which works well with MultiThreaded Application.

Answer 6

Its not necessary to lock/synchronize getters and setters of primitive types - tagging them as volatile is sufficient in most cases (apart from double and long as you mention)

As one of the previous posts mentions, you need to be aware of read and update sequences, something like incrementI(int num) which will likely invoke getI() and setI() - in this case you could add a 'synchronized incrementI(int num)' method to your Example class. Locking is then done at a higher level, reducing the need for separate read and write locks and is OO friendly as data and behaviour stay together. This technique is even more useful if you are reading/updating several fields at once.

Though if you are simply, reading/writing/updating one field at a time, then the AtomicXX classes are more appropriate