CODEWITHSUNDEEP
javamultithreadingreaderwriterlock
codereviewanskquestions
codereviewanskquestions

Reputation: 13998

How to synchronize getter and setter in threading

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?

Upvotes: 3

Views: 2343

Answers (5)

Glen Best
Glen Best

Reputation: 23115

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".

Upvotes: 1

Steve Kuo
Steve Kuo

Reputation: 63104

Make ratio final and it will be thread safe.

Upvotes: -1

jtahlborn
jtahlborn

Reputation: 53694

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

Upvotes: 1

Juned Ahsan
Juned Ahsan

Reputation: 68715

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!

Upvotes: 0

greedybuddha
greedybuddha

Reputation: 7507

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.

Upvotes: 1

Related Questions