0

I have a class that has the object "Card". This class keeps checking to see if the object is not null anymore. Only one other thread can update this object. Should I just do it like the code below? Use volatile?Syncronized? lock (which I dont know how to use really)? What do you recommend as easiest solution?

Class A{

 public Card myCard = null;


 public void keepCheck(){

    while(myCard == null){
       Thread.sleep(100)
    }
    //value updated
     callAnotherMethod();
 }

Another thread has following:

public void run(){
     a.myCard = new Card(5);
 }

What do you suggest?

Snake
  • 14,228
  • 27
  • 117
  • 250
  • You should use a proper wait event (see the [Guarded Block](http://docs.oracle.com/javase/tutorial/essential/concurrency/guardmeth.html) tutorial), otherwise you run the risk of the "watching" thread seeing the reference before it sees completely initialized member fields of the `Card`. – Jason C Mar 15 '14 at 07:47
  • Of course that problem can be completely avoided if the creating thread creates the reference *then* assigns myCard to equal that reference. – Gabe Sechan Mar 15 '14 at 07:52
  • @GabeSechan The JLS only makes that guarantee for `final` fields of the object (even in the case where you're creating then assigning). The *only* way you can guarantee that another thread sees the up-to-date state of the object is to use `synchronized`, which has happens-before guarantees about actions that take place up to that point. The issue is not with one thread getting access to the reference before the constructor returns - that's impossible of course; the issue relates to execution/visibility reordering and also low level caching of out-of-date data (esp. on multicore systems). – Jason C Mar 15 '14 at 08:02
  • @JasonC A reference is basically a double pointer (doubled so it can be moved in memory). Assignment of the variable is atomic- all fields update at once because all you're doing is updating a single pointer in memory. It works just fine. Now if the thread that's using it wants to do more than a null check it will need to use more sophisticated locking like wait or a semaphore. – Gabe Sechan Mar 15 '14 at 08:09
  • @JasonC Worries about multicore caching are invalid in this specific case where one thread is waiting for the variable to be non-null- if it reads an out of date version it will simply get it the next time it checks. This is a very frequent pattern in rendering code- one thread renders an object to a bitmap, and the drawing thread will copy the current value of the reference to the bitmap and use it to draw to the screen. This allows lockless drawing. – Gabe Sechan Mar 15 '14 at 08:11
  • @GabeSechan *"Now if the thread that's using it wants to do more than a null check"* I've assumed that `callAnotherMethod()` will be using the data; that may not be a correct assumption. – Jason C Mar 15 '14 at 08:21
  • interesting discussion. I have got alot to learn – Snake Mar 15 '14 at 16:36

3 Answers3

0

The variable needs to be volatile when modifying from a different thread if you intend to poll for it, but a better solution is to use wait()/notify() or even a Semaphore to keep your other thread sleeping until myCard variable is initialized.

Kayaman
  • 72,141
  • 5
  • 83
  • 121
0

Looks like you have a classic producer/consumer case.

You can handle this case using wait()/notify() methods. See here for an example: How to use wait and notify in Java?

Or here, for more examples: http://www.programcreek.com/2009/02/notify-and-wait-example/

Community
  • 1
  • 1
Hakan Serce
  • 11,198
  • 3
  • 29
  • 48
0

You should use a proper wait event (see the Guarded Block tutorial), otherwise you run the risk of the "watching" thread seeing the reference before it sees completely initialized member fields of the Card. Also wait() will allow the thread to sleep instead of sucking up CPU in a tight while loop.

For example:

Class A {

    private final Object cardMonitor = new Object();
    private volatile Card myCard;

    public void keepCheck () {
        synchronized (cardMonitor) {
            while (myCard == null) {
                try {
                    cardMonitor.wait();
                } catch (InterruptedException x) {
                    // either abort or ignore, your choice
                }
            }
        }
        callAnotherMethod();
    }

    public void run () {
        synchronized (cardMonitor) {
            myCard = new Card(5);
            cardMonitor.notifyAll();
        }
    }

}

I made myCard private in the above example. I do recommend avoiding lots of public fields in a case like this, as the code could end up getting messy fast.

Also note that you do not need cardMonitor -- you could use the A itself, but having a separate monitor object lets you have finer control over synchronization.

Beware, with the above implementation, if run() is called while callAnotherMethod() is executing, it will change myCard which may break callAnotherMethod() (which you do not show). Moving callAnotherMethod() inside the synchronized block is one possible solution, but you have to decide what the appropriate strategy is there given your requirements.

Jason C
  • 38,729
  • 14
  • 126
  • 182
  • by the way, why do you have while(card ==null) when you have the wait! should'nt it just be "if" since you wait for the wakeup once – Snake Mar 15 '14 at 16:41
  • @Snake quick comment from phone - two things: 1. If run notifies before keepcheck waits, keepcheck will miss notification. So we check condition first (in this case, condition==true implies notify was already called). 2. Threads can wake up spuriously (or via interrupt()) even if condition isn't true. Google "spurious wakeups". – Jason C Mar 15 '14 at 19:59