CODEWITHSUNDEEP
javamultithreadingsynchronizationsynchronized
James Anderson
James Anderson

Reputation: 203

Why is this not synchronized correctly?

Hi all I have this code:

public class ThreadTester {
    public static void main(String args[]) {
        Counter c = new Counter();
        for (int i = 0; i < 10; i++) {
            MyThread a = new MyThread(c);
            MyThread b = new MyThread(c);
            a.start();
            b.start();
        }   
        System.out.println("The value of the balance is " + c.getVal());
    }
}

class MyThread extends Thread {
    private Counter c;
    public MyThread(Counter c){ this.c = c; }
    public void run(){ s.increment(); }
}

class Counter {
    private int i = 100;
    public synchronized void increment(){ i++; }
    public synchronized int getVal(){ return i; }
}

Now I thought that this should give the desired result of 120 - however the result seems to fluctuate between 115 and 120. If I add a Thread.sleep(1) after b.start() I always get the desired result of 120. Why does this happen?

It's really been confusing me and I'd appreciate any help I could get, Thanks

Upvotes: 1

Views: 110

Answers (5)

xagyg
xagyg

Reputation: 9721

You are printing the result potentially before the threads are finished (which is why the results vary). You need to wait until all threads have completed before printing the result.

Restructure your main method as follows:

public static void main(String args[]) {
    Counter c = new Counter();
    MyThread[] a = MyThread[20];
    for (int i = 0; i < 20; i++) {
        a[i] = new MyThread(c);
        a[i].start();
    }
    for (int i = 0; i < 20; i++) {
        a[i].join();        
    }
    System.out.println("The value of the balance is " + c.getVal());
}

Firstly, I am looping 20 times since your loop iterated 10 times whilst creating two threads (so you were creating 20 threads too). You need to hold on to the references (via the a array) so that the main thread can wait until all the threads have completed (with join). When they have all completed, the correct result is returned.

Upvotes: 0

goblinjuice
goblinjuice

Reputation: 3214

The reason:

The third Thread (which executes the main()) function gets to the following statement right after starting Threads a and b in random order and that's why you get unpredictable results.

    System.out.println("The value of the balance is " + c.getVal());

Next Question:

If I add a Thread.sleep(1) after b.start() I always get the desired result of 120. Why does this happen?

This happens because you stop the main Thread long enough (1 second is a long time in CPU world) to allow Threads a and b to finish.

Solution: Make the main Thread wait until both Threads a and b have finished. One way:

    Counter c = new Counter();
    for (int i = 0; i < 10; i++) {
        MyThread a = new MyThread(c);
        MyThread b = new MyThread(c);
        a.start();
        b.start();
    }   
    a.join(); // wait for thread a to finish
    b.join(); // wait for thread b to finish

    System.out.println("The value of the balance is " + c.getVal());

Upvotes: 0

PaulProgrammer
PaulProgrammer

Reputation: 17720

Because not all the threads have completed their increment task by the time you get to the System.out. Injecting the sleep allows the threads to finish before getting to the output.

Upvotes: 0

JB Nizet
JB Nizet

Reputation: 692281

You're printing the value of the counter after having started all the threads, and not after all the threads have completed.

Use Thread.join() on all the threads you have started to wait until they've completed, and then print the value. Or use a CountDownLatch. Sleeping gives you the correct result by accident. It allows all the threads to complete, but only because they have so few things to do that sleeping for 1 millisecond is sufficient.

Upvotes: 3

Brian Roach
Brian Roach

Reputation: 76918

Because threads run in parallel.

You're printing c.getVal() in the main thread before one or more of your other threads has incremented it.

When you sleep, you're allowing the other threads enough time to complete, then printing.

Upvotes: 1

Related Questions