jmhostalet
jmhostalet

Reputation: 4649

Java: Is this approach thread safe?

I have a Thread and I need to set when it is listening or in standby, for that I've defined

public static enum ListenerState { STAND_BY, LISTENING }; 

and a method

public void setState(ListenerState state){
    this.state = state;
}

now, in the main loop I check the state in this way

@Override
public void run() {
    while (!Thread.interrupted()) {
        try {
            if (state==ListenerState.LISTENING){
               // do my job
            }
            else{
                Thread.sleep(300);
            }
        }
    }
}

Is this approach thread-safe ?

Upvotes: 1

Views: 82

Answers (3)

Adem
Adem

Reputation: 9429

if state can be changed or read by different thread, then you need to synronize block for reading and writing methods. or as a better way, use AtomicBoolean. it is perfect object to get rid of syncronize block and make it thread safe

https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/atomic/AtomicBoolean.html

Upvotes: 1

win_wave
win_wave

Reputation: 1508

No, do like that:

class MyThread implements Runnable {

    private volatile ListenerState state;

    public synchronized void setState(ListenerState state){
       this.state = state;
    }

    @Override
    public void run() {
       while (true) {
         try {
            if (state==ListenerState.LISTENING){
              // do my job
            } else{
              Thread.sleep(300);
            }
          } catch (IterruptedException ex){
            return; 
          }
        }
    }
}

Upvotes: 3

DontRelaX
DontRelaX

Reputation: 754

You can find your answer here: Do I need to add some locks or synchronization if there is only one thread writing and several threads reading?

in one word: better to add volatile keyword to state.

Upvotes: 2

Related Questions