CODEWITHSUNDEEP
javamultithreading
Javed Solkar
Javed Solkar

Reputation: 164

2 Thread access shared resource without synchronized keyword, still works fine

public class Worker {
private int count = 0;                  //shared resource

public static void main(String[] args) {
    Worker w = new Worker();
    w.doWork();
}

public void doWork() {

    Thread thread1 = new Thread(new Runnable() {   //Thread incrementing count 10000 times
        public void run() {
            for (int i = 0; i < 10000; i++) {
                count++;          //Not Atomic operation

            }

        }
    });

    Thread thread2 = new Thread(new Runnable() {    //Thread incrementing count 10000 times
        public void run() {
            for (int i = 0; i < 10000; i++) {
                count++;           //Not Atomic operation
            }

        }
    });
    thread1.start();
    thread2.start();

    try {//halts main thread so that both thread race to increment count
        thread1.join();
        thread2.join();
    } catch (InterruptedException e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
    }

    System.out.println("Count is: " + count); 
}
}

I copied this code from Cave Of Programming. Every time I run this code, I get result as 20000 without using synchronized keyword, which is unexpected. Is the code correct ? My OS schedules both the thread correctly? (I know this is weird Question)

I am expecting unwanted result every time I run the code.

Upvotes: 0

Views: 113

Answers (6)

acearch
acearch

Reputation: 342

@Javed.. I hope you have seen the join fundamental in threads.. Looks like Join is doing the job for you and also for me.. I have run your program 10 times and have always got 20000. Also I tried a SOP such as Thread.currentThread().getName() before the count++ in both the threads and then ran the program. I found that there is no interlacing between the thread1 and thread2...All SOPs for Thread1 are displayed and then the SOPs for Thread2 are displayed hence there is no race.. Thread1 finishes its work and then Thread2 does its job hence 20000 will always be the result.

Upvotes: 0

Victor
Victor

Reputation: 3688

The code is incorrect. The correct result is a coincidence.

First count++ is not atomic.

Second, a task might run till it ends and the other could run afterwards - no restrictions here. The schedule can decide to give so much time that a task actually finishes and than it switches or it is just giving enough time that count++ is actually behaving like atomic(just because of the time between switches). But the could could provide different results. In the end 2000 is a mere and happy coincidence.

Try to increase the loop size, in order to see different results. In summary, the code is correct to provide incorrect results,as you expected, just expand you experiment.

Upvotes: 0

shanmuga
shanmuga

Reputation: 4499

It will behave differently depending on OS and load on the system.

I executed the same code 3 times and got the different results each time

$ java Worker
Count is: 10437
$ java Worker
Count is: 10395
$ java Worker
Count is: 10684

I used Ubuntu 14 (x64) with OpenJDK-7

Upvotes: 1

HJK
HJK

Reputation: 1382

First of all count++ is not atomic. Secondly though you got the answer correct 2000, we cannot guarantee that it would be correct always.

Reason would be as follows. Operation count++ is treated as a 3 step process

 1. Fetch the count value
 2. Increment the count value
 3. assign the incremented value back to count variable

So when thread1 is reading, Thread 2 might increment the value. Still thread 1 has ole value, and overrding the value incrmented by thread2

I would be definitely correct if you make the count++ atomic. For this you can use as follows

 AtomicInteger count = new AtomicInteger();

and you could increment it using

 count.incrementAndGet();

Upvotes: 1

Qadir Hussain
Qadir Hussain

Reputation: 1263

As per your code your for loops will executed 10000 times in both the cased so its obvious that it will give you 20000 as result.

Advice: Try printing intermediate value of count

Thread thread1 = new Thread(new Runnable() {   //Thread incrementing count 10000 times
    public void run() {
        for (int i = 0; i < 10000; i++) {
            count++;          //Not Atomic operation
            System.out.println(count);
        }

    }
});

Then you will realize the difference of with synchronized and without synchronized.

Upvotes: 0

Mattias
Mattias

Reputation: 1110

The expected output is that both of the threads increment the count variable from 0 by 10000 each, so the expected output is indeed 0 + 10000 + 10000 = 20000.

However, since count++ is not an atomic operation, this is not deterministic. Run it more times and you may get different results.

Upvotes: 0

Related Questions