How to synchronize getter and setter in threading

Question

public class IntermediateMessage {

    private final ReentrantReadWriteLock readWriteLock = new ReentrantReadWriteLock();
    private final Lock read = readWriteLock.readLock();
    private final Lock write = readWriteLock.writeLock();

    private volatile double ratio;

    public IntermediateMessage(){
        this.ratio=1.0d;
    }

    public IntermediateMessage(double ratio){
        this.ratio = ratio;
    }

    public double getRatio(){
        read.lock();
        try{
            return this.ratio;
        }
        finally{
            read.unlock();
        }
    }

    public void setRatio(double ratio){ 
        write.lock();
        try{
            this.ratio = ratio;
        }
        finally{
            write.unlock();
        }
    }
}

I have this object. I have an instance of this object in my application and one thread is writing to the ratio variable while the other threads are reading the ratio. Is this correct way to protect the ratio variable? Do I need to declare ratio as volatile?

sorry. my bad. I updated my question. I just wanted to make sure if this is necessary to sue read/write lock with volatile declaration — codereviewanskquestions, May 29 '13 at 01:17

score 1 · Answer 1 · answered May 29 '13 at 01:23

You are doing some overkill on the synchronization that is going to cause some inefficiency.

The java keyword "volatile" means that variable won't be cached, and that it will have synchronized access for multiple threads.

So you are locking a variable that is already by default synchronized.

So you should either remove the volatile keyword, or remove the reentrant locks. Probably the volatile as you will be more efficient with multiple reads the way you are currently synchronizing.

score 1 · Answer 2 · answered May 29 '13 at 01:35

1

For reading/writing a primitive value, volatile alone is sufficient.

answered May 29 '13 at 01:35

jtahlborn

52,909
5
76
118

score 1 · Accepted Answer · answered May 29 '13 at 04:59

1

Do you need locking at all? Most likely not, according to the limited requirements you've described. But read this to be sure...

You have just one thread writing.
- This means that the variable value can never be "out of date" due to competing writers "clobbering" one another (no possible race condition). So no locking is required to give integrity when considering the individual variable in isolation.
You have not mentioned whether some form of atomic, consistent modification of multiple variables is required. I assume it isn't.
- IF ratio must always be consistent with other variables (e.g. in other objects) - i.e. if a set of variables must change in synchrony as a group with no one reading just part of the changes - then locking is required to give atomic consistency to the set of variables. Then consistent variables must be modified together within in a single locked region and readers must obtain the same lock before reading any of these set of variables (waiting in a blocked state, if necessary).
- IF ratio can be changed at any time as a lone variable and need not be kept consistent with other variables, then no locking is required give atomic consistency to a set of variables.

Do you need the volatile modifier? Well, yes!

You have multiple threads reading.
The variable can change at any moment, including an instant before it's about to be read.
The volatile modifier is used in multi-threaded apps to guarantee that the value read by "readers" always matches the value written by "writers".

answered May 29 '13 at 04:59

Glen Best

22,769
3
58
74

also, volatile is required for long/double because they are not guaranteed to be written atomically otherwise. – jtahlborn May 29 '13 at 11:51
That's right in general, but not a problem here. As stated, there's a single writer thread. So competing no-atomic writes (aka "clobbering") not an issue. – Glen Best May 29 '13 at 14:34
the problem isn't with the writing, it's with the reading (aka a reader could see a partially updated value). so, yes, it is relevant here. – jtahlborn May 29 '13 at 23:26
Thanks :). Following's moot: volatile required anyway for threads to see same value. But there's never "partial update" to long/double. Bytecode always updates these atomically (across all bits), doesn't set partial bits. Volatile ensures new values stored directly to location visible to all threads, not to temporarily fast register/cache of one thread or process. Read/write to volatile also moves other variables of the class to thread-visible storage. See JLS: http://docs.oracle.com/javase/specs/jls/se7/html/jls-8.html#jls-8.3.1.4 and "Java Concurrency In Practice". Hope that helps :) – Glen Best May 30 '13 at 01:14
Note, declaring `volatile int a`, when writing from a single thread makes `a++` appear to be atomic (as mentioned in Concurrency in Practice). But `b=a` is always atomic. :) – Glen Best May 30 '13 at 01:22
yes, the point is moot due to the requirement of volatile for other reasons. however, my original point still stands, see JLS: http://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html#jls-17.7 – jtahlborn May 30 '13 at 03:03
Thanks, I didn't know that! So technically there can be partial updates. Under conditions when multiple threads can read differing/inconsistent values, they can also read 64 bits with (2 x inconsistent 32 bits). Probably not widely discussed, because volatile always needed there. – Glen Best May 30 '13 at 07:22

score 0 · Answer 4 · answered May 29 '13 at 01:33

Provided two threads are trying to read and write on the same object and you want the data integrity to be mantained. Just make your getter and setter synchronized. When a method is synchonized, only one thread will be able to call a synchronize method. While one thread is executing one of the synchronized method, no other thread will be able to call any of the synchronized method. So in your case if you have your get & set method synchronized, you can be sure if a thread is reading/writing no other thread can do the reading/writing.

Hope it helps!

score -1 · Answer 5 · answered May 29 '13 at 03:20

-1

Make ratio final and it will be thread safe.

answered May 29 '13 at 03:20

Steve Kuo

61,876
75
195
257

That won't do it. Final just means that you can't change the definition in subclasses of the current class. ratio is of type int - and not some class designed to be immutable. static final would make it threadsafe - but the Q specifically says that ratio is to be mutable. – Glen Best May 29 '13 at 05:05
All final fields are guaranteed to be thread safe. This applies to static and non-static fields. It has nothing to do with subclasses. – Steve Kuo May 30 '13 at 00:12
Thanks! Scrub the mention of `static` - on the wrong track there. `final` will make the `int` immutable and hence, threadsafe. Then it could only be set once during object initialisation; `static final` could only be set once during class initialisation. But poster wants a mutable object, so `final` won't work. – Glen Best May 30 '13 at 01:35
I was nudging the question poster towards immutability. It would be better to have an immutable message object that gets queued (see JAL's answer). – Steve Kuo May 30 '13 at 03:43

How to synchronize getter and setter in threading

5 Answers5