views:

259

answers:

4

I have a function that launches a couple of threads (it's a test function) and one of the threads mutates the state of a variable. Since local variables cannot be marked volatile, I would assume that multiple threads in that method will always have the updated state of the variable. Is this correct? Here is the sample code

public void someMethod() {
   MutableBoolean mb = new MutableBoolean(false);
   Thread t1 = new Thread() {
       public void run() {
           while (someCondition) {
              if ( mb.getValue() ) {
                 ...do something
              }
           }
       }  
   }
   t1.start();

   Thread t2 = new Thread() {
         public void run() {
             if ( someCondition ) {
                mb.setValue(true);
             }
         }
   }
   t2.start();  

   ...wait for the threads to complete

}

+6  A: 

Unless MutableBoolean uses a lock in it's set/get value methods, or it's an atomic operation then this isn't threadsafe.

getValue could be reading the value while setValue is updating it. You might get away with this with a boolean, but for any more complex type it'll probably fail.

Put a lock around the access to shared state to make it thread safe.

Glen
Thanks. I knew I needed to make it final too (though I don't think you made that comment). This was just example code and not necessarily fully compiling.
Jeff Storey
A: 

To have the inter-thread visibility, volatile is not the only option. Any synchronization (lock) will also work.

In this case, the MutableBoolean could be a thread-safe class, so that would do the job.


As said danben, the MutableBoolean may need to be final to be passed to the functions.

Did you test your code in an IDE before posting?

KLE
A: 

mb must be defined as final or otherwise the code will not compile (an anonymous class can only access the declaring scope's final variables). A final variable cannot change its value so technically both t1 and t2 will always see the same value at mb. For the reason, the issue of mb being volatile or not is irrelevant.

Your question is not about the state of the mb object itself (the values held by the fields of the MutableBoolean object). For these fields you can use volatile and/or synchronized methods to ensure thread-safety.

Itay
A: 

Previous posters are correct that this code won't compile without making mb final. For reference types, final means that you can't change what object the variable refers to. It does not mean that you can't change the state of that object. So, you can't make mb refer to a different instance of MutableBoolean, but setValue(...) will behave as always. MutableBoolean.getValue() and MutableBoolean.setValue(...) should be synchronized, so that one thread can see each other's changes.

Note that there is no guarantee about what order the statements in the two new threads will run in. Either t2 or t1 might run first, or instructions might be interleaved.

None of this is really affected by the fact that mb is a local variable, except the requirement for it to be final.

See Java Concurrency in Practice for the definitive word on this topic.

cbare