CODEWITHSUNDEEP
javaconcurrency
performanceuser
performanceuser

Reputation: 2893

Is this java class thread safe? Concurrent read and write

Is the following class thread safe? I am worrying about concurrent read and write the initialized variable. If it is not thread safe, how to make it thread safe?

  1. I know convert methodA to synchronized will help, but I don't want to do this
  2. How about add volatile keywork to "initialized" variable?
public class A {

    private boolean initialized;

    public synchronized void init(String configFilePath) {
        if (initialized) {
            return;
        }

        initialized = true;
    }

    public void methodA() {
        if (!initialized) {
            throw new ConfigurationException()
        }
    }
}

Update1: The initialized variable will only be modified once in init method, all other methods will only ready. If that is the case, adding volatile to initialized will make it thread safe, is that correct?

Upvotes: 1

Views: 377

Answers (3)

xagyg
xagyg

Reputation: 9721

@HotLicks is 100% correct. Any question regarding concurrency needs to provide context. Here's why:

Let's assume a class has been written to be "safe" (ignoring the OPs class for the moment). If you are synchronizing on an instance variable and the instance of the class is shared by many threads, then it will be thread-safe. However, if multiple instances of the class can be created (by the different threads), and they potentially modify static variables/state, then it will only be thread-safe if you synchronize on a static (i.e. class) variable.

In summary:

  1. If sharing single instance between threads, then lock on instance variable
  2. If threads are creating instances of the "safe" class AND static state is potentially modified by those threads, then must lock on static (class) variable

Upvotes: 0

AlexWien
AlexWien

Reputation: 28767

No its not thread safe. you have to syncronize.

Upvotes: 0

Ted Hopp
Ted Hopp

Reputation: 234857

No, it is not thread safe. The init routine could be in the middle of setting initialized when methodA is called. Since methodA is not synchronized, there's nothing preventing a race between executing initialized = true and the read in if( !initialized). In fact, the write could even have happened but simply not yet propagated to the thread that called methodA

Adding volatile to initialized would help with the value propagation problem, but not with the first.

For more info on this, I recommend Brian Goetz's article Managing Volatility.

Upvotes: 3

Related Questions