CODEWITHSUNDEEP
javamultithreadingthread-safetyvolatileconditional-operator
mogli
mogli

Reputation: 1609

Is it safe to use conditional operators with volatile primitives in multithreaded applications

In the below code listings, are Statement 1 and Statement 2 thread safe or not? They are using the VolatileIntWrapper.

If they not thread safe, which statements need to be wrapped in synchronized block?

public class Demo {

    public static void main(String[] args) {

        VolatileIntWrapper volatileIntWrapper = new VolatileIntWrapper() ;

        for(int i = 1 ; i <= 5 ; ++i){
            new ModifyWrapperIntValue(volatileIntWrapper).start() ; 
        }
    }
}

class VolatileIntWrapper{
    public volatile int value = 0 ;
}

class ModifyWrapperIntValue extends Thread{

    private VolatileIntWrapper wrapper ;
    private int counter = 0 ;

    public ModifyWrapperIntValue(VolatileIntWrapper viw) {
        this.wrapper = viw ;
    }

    @Override
    public void run() {

        //randomly increments or decrements VolatileIntWrapper primitive int value

        //we can use below statement also, if value in VolatileIntWrapper is private
        // wrapper.getValue() instead of wrapper.value
        //but, as per my understanding, it will add more complexity to logic(might be requires additional synchronized statements),
        //so, for simplicity, we declared it public


        //Statement 1
        while(wrapper.value > -1500 && wrapper.value < 1500){
            ++counter ;
            int randomValue = (int) (Math.random() * 2) ;

            //Statement 2
            wrapper.value += (randomValue == 0) ?   1       :       -1 ;
        }

        System.out.println("Executed " + counter + " times...");
    }
}

Upvotes: 1

Views: 199

Answers (4)

Alexandr
Alexandr

Reputation: 9525

Java In Concurrencty says the following criteria need to be met touse volatile variables:

1.  Writes to the variable do not depend on its current value, or you can ensure that only a single thread ever updates the value;

2.  The variable does not participate in invariants with other state variables; and

3.  Locking is not required for any other reason while the variable is being accessed.

Upvotes: 0

Gray
Gray

Reputation: 116908

The volatile keyword provides a memory barrier for both reading and writing a field. That means that multiple threads can access the field and be guaranteed to read the most current value and their writes are guaranteed to be seen by other threads.

What volatile does not do is provide any guarantees around the order of operations -- especially when you have multiple read and write statements. In your code you are accessing the volatile int a couple of places in your loop:

    while(wrapper.value > -1500 && wrapper.value < 1500){
        ...
        wrapper.value += (randomValue == 0) ?   1       :       -1 ;
    }

There are no guarantees as to the order of operations here. Immediately after thread A tests the value > -1500, another thread might change it before thread A can test value < 1500. Or thread A might do both tests, then thread B might do both tests, then thread A would assign the value, and then thread B would assign the value. That is the nature of multithreading race conditions.

The while loop is the section of code that I suspect would be considered have a bug unless you synchronize around it. You should do something like the following. Once you are synchronizing that section, the synchronized keyword provides the memory barrier itself and so the volatile keyword is unnecessary.

   synchronized (wrapper) {
       while (...) {
         ...
       }
   }

Upvotes: 4

GingerHead
GingerHead

Reputation: 8230

The question needs the following interpretation:

The thread you are using is safe and you are reading your primitive value as it is intended.

there is a specific term to use synchronize blocks on the primitive field, but you need to do the following:

  • Use getter and setters of your field.
  • Put synchronize in the both accessors and voila.

Upvotes: 0

Peter Lawrey
Peter Lawrey

Reputation: 533730

It is safe to use a volatile field once and only once. (A read and a write counts as twice)

You are using the field a total of four times so you have three places for a race condition.

The problem with this example is it is faster and simpler to be performed single threaded, so anything you do with it in a multi-threaded way will appear unnatural and inefficient.

Upvotes: 2

Related Questions