3

Here the code

class Aux implements Runnable {

    private Boolean isOn = false;
    private String statusMessage;
    private final Object lock;

    public Aux(String message, Object lock) {
        this.lock = lock;
        this.statusMessage = message;
    }

    @Override
    public void run() {
        for (;;) {
            synchronized (lock) {

                if (isOn && "left".equals(this.statusMessage)) {
                    isOn = false;
                    System.out.println(statusMessage);
                } else if (!isOn && "right".equals(this.statusMessage)) {
                    isOn = true;
                    System.out.println(statusMessage);
                }

                if ("left".equals(this.statusMessage)) {
                    System.out.println("left " + isOn);
                }

            }
        }
    }
}

public class Question {
    public static void main(String [] args) {
        Object lock = new Object();

        new Thread(new Aux("left", lock)).start();
        new Thread(new Aux("right", lock)).start();
    }
}

In this code I expect to see:

left, right, left right and so on,

but when Thread with "left" message changes isOn to false, Thread with "right" message don't see it and I get ("right true" and "left false" console messages), left thread don't get isOn in true, but right Thread can't change it cause it always see old isOn value (true).

When i add volatile modifier to isOn nothing changes, but if I change isOn to some class with boolean field and change this field then threads are see changes and it works fine

Thanks in advance.

andymur
  • 147
  • 1
  • 9

1 Answers1

7

isOn is a thread local variable, so when one thread modifies it, the other thread won't see the changes. When you use a class with a boolean field, you're presumably giving each thread the same instance of the class, meaning that the threads will see changes made by the other threads. However, you create two instances of Aux instead of using just one.

If you want it to work on two different instances, try adding the static modifier (along with the volatile modifier) to the Boolean field; otherwise, don't create a second Aux.

Steve P.
  • 14,489
  • 8
  • 42
  • 72
Zim-Zam O'Pootertoot
  • 17,888
  • 4
  • 41
  • 69
  • 2
    @Brian Roach By "just use the class with a boolean field" I'm referring to the second clause of the second to last paragraph of his question - he's using a class like `class MyBoolean { public boolean isOn; }` and giving the same `MyBoolean` instance to the two threads – Zim-Zam O'Pootertoot Jun 29 '13 at 22:28
  • 2
    oic - right, carry on :) (Beer involved ... need to walk away from the SO) – Brian Roach Jun 29 '13 at 22:30
  • Does using synchronized(otherThread){otherThread.booleVar=true;} work? – huseyin tugrul buyukisik Jun 29 '13 at 22:45
  • @huseyintugrulbuyukisik why would you want to do that instead of making the boolean field static or just using the same instance of `Aux`? – Steve P. Jun 29 '13 at 22:49
  • @huseyin tugrul buyukisik Yes, as long as both threads sync their `isOn` variables. `synchronized(lock) { this.isOn = false; otherThread.isOn = false; }` would be fine, but I'd prefer to use a shared field - more efficient and less risk of error – Zim-Zam O'Pootertoot Jun 29 '13 at 22:49
  • @SteveP. Thank you, when I using one Aux instance while constructing threads then everything's ok. If we use static variable it works fine, it seems to be clear for me now with one question, why threads don't share primitive type or wrappers (int, boolean etc) when we pass the same object before starting it, but share other object. `private Boolean isOn = false; private final Object lock; private SomeObj someObject; public Aux(SomeObj someObject, Object lock) { this.lock = lock; this.someObject = someObject; }` in that case this.someObject will be shared – andymur Jun 30 '13 at 07:01
  • 1
    @andymur Threads don't share primitive types because Java is pass by value - when you pass an `int` to two threads then each thread gets its own copy of the `int`. Primitive wrappers are immutable, so e.g. when you pass an `Integer` to two threads then each thread gets a tread-local copy of the same `Integer` reference, but then when one thread overwrites the `Integer` it only overwrites its thread-local copy, which the other thread won't see. – Zim-Zam O'Pootertoot Jun 30 '13 at 13:33
  • @andymur If you want mutable primitive wrappers, then you can use the `Atomic[Primitive]` classes, such as [AtomicBoolean](http://docs.oracle.com/javase/6/docs/api/java/util/concurrent/atomic/AtomicBoolean.html) or [AtomicInteger](http://docs.oracle.com/javase/6/docs/api/java/util/concurrent/atomic/AtomicInteger.html). If you pass the same `Atomic[Primitive]` object to two threads and one thread calls `set(newValue)` on the object then the other thread will see the change when it calls `get()` – Zim-Zam O'Pootertoot Jun 30 '13 at 16:34