CODEWITHSUNDEEP
javamultithreadingthread-safety
Jarod
Jarod

Reputation: 1712

Are these classes thread-safe?

First class:

class Main1 {
    private ExecutorService service = Executors.newFixedThreadPool(4);

    public static void main(String[] args) {
       Main1 m = new Main1();
       m.start();
    }

    public void start() {
        final MyObject obj = new MyObject(); 
        obj.doSomeCalculation();//  after this point not to modify obj in main thread

        service.submit(new Runnable(){
            public void run() {
                    obj.doSomething(); // is it threadsafe doing this?
               }
        });
    }
}

Second class:

class Main2 {
    private ExecutorService service = Executors.newFixedThreadPool(4);

    public static void main(String[] args) {
        Main2 m = new Main2();
        m.start();
    }

    public void start() {
        class Job implements Runnable {
           public MyObject obj;

            public void run() {
                obj.doSomething(); // is it threadsafe doing this?
            }
        }

        Job job = new Job();
        job.obj.doSomeCalculation(); // after this point not to modify obj in main thread
        service.submit(job);
    }
}

Are Main1 and Main2 threadsafe? Does Main1 and Main2 make different sense to thread-safety?

update: neither doSomeCalulation() nor doSomething() don't have any lock or synchronized block. I want to known whether doSomething() could always read the correct states that doSomeCalculation() change to obj

Upvotes: 1

Views: 256

Answers (2)

Stephen C
Stephen C

Reputation: 718788

Are Main1, Main2 threadsafe?

In the Main1 case, the thread-safety of the application depends on whether MyObject is thread-safe and whether any other threads doing things with it. However, the obj.doSomething(); statement is thread-safe assuming nothing else is changing the object

In fact, the obj.doSomething(); statement doesn't use the variable in the enclosing class. Instead, the value of that variable is passed to the inner class in a hidden constructor argument. The other thing that makes this thread-safe is that there is an implicit synchronization between the parent and child threads when a new thread is created. (Reference - JLS 17.4.4 Synchronization Order) These two facts combined mean that the Runnable.run() method will get the correct reference, and that the child thread will see the state of the object at the synchronization point (or later).

In the Main2 case, the same applies. In this case you are merely doing explicitly (more or less) what is happening implicitly in the Main1 case.

UPDATE - the above reasoning applies even if you mutate the object in the parent thread (as per your updated question) before passing it to the child thread ... because of the implicit synchronization that I mentioned. (However, if the parent thread were to change MyObject after the submit() call, you'd run into thread-safety problems.)

Does Main1 and Main2 make different sense?

I don't know what you are asking. If you are asking if there is any benefit in using an inner class rather than an anonymous inner class ... in this case the answer is no. They behave the same with respect to thread-safety.

Actually, the Main1 version is better because its is simpler, more readable (to an experienced Java developer), and more robust. The Main2 class exposes the obj field for other code to potentially access or even update. That's bad style. You could fix that, but only by adding more code ... which brings us back to simplicity / readability.

Upvotes: 3

shams
shams

Reputation: 3508

The way your code is structured, you submit your job (in both cases) only after you have already performed the calculation above. So there is no chance of those two actions happening in parallel and so there are no data races.

However, if you were to perform your calculations after submitting the job/Runnable to the executor service, then those two calculations could happen in parallel and there could be a data race.

Job job = new Job();
service.submit(job);
// Now there is a data race!!!
job.obj = ...// do some calculation, and after this point not to modify obj in main thread

Upvotes: 1

Related Questions