Question

I start learning some java concurrency concept ans put in use. But one of this piece of code is beyond my understanding.

public class Count { 
    private int count = 0; 
    public synchronized void setCount(int count) { 
        this.count = count; 
    } 
    public synchronized int getCount() { 
        return count; 
    } 
} 
class CountRunner extends Thread { 
    Count count; 
    public CountRunner(Count count) { 
        this.count = count; 
    } 
    public void run() { 
        for (int i = 1; i <= 1000; i++) { 
            count.setCount(count.getCount() + 1); 
        } 
    } 
} 
class TestCount { 
    public static void main(String[] args) throws Exception { 
        Count count = new Count(); 
        CountRunner runnerA = new CountRunner(count); 
        CountRunner runnerB = new CountRunner(count); 
        runnerA.start(); 
        runnerB.start();         
        runnerA.join(); //join statement here 
        runnerB.join(); 
        System.out.println("count.getCount = " + count.getCount()); 
    } 
} 
Question:
1. The result is a little less than 2000 for many times, why ?
2. if delete 2 join() statement, why count.getCount = 451,even less ?
3. i think there will be no effect of deleting the join() statements,
because i already have Synchronized method to lock one object to one thread each time ? 
So, What's the point of using Synchronized and join() ?
Was it helpful?

Solution

It is pretty simple. You call the setCount method by calling the getCount + 1. Before entering the method, the runtime evaluates the getCount (synchronized), but you don't hold the lock when leaving getCount and entering setCount and other threads can enter an call getCount. So every now and then two (or more depending on how many threads you create) threads will have the same value in the getCount. Imagine thread A enters and receives the value 1 in getCount. The runtime yields it's execution to tread B which calls getCount and receives the same value 1. Thread B sets the value to 1 and makes another 50 runs, so your count would be 50 at that stage. The runtime yields the execution to thread A which calls setCount with 1 (remember it didn't manage to call setCount and yielded it's exec). Now A sets the value to 1 (which is wrong).

Change you run implementation like this:

public void run() { 
    for (int i = 1; i <= 1000; i++) {
      synchronized(count){ 
        count.setCount(count.getCount() + 1); 
      }
    } 
} 

OTHER TIPS

  1. If you break the line

    count.setCount(count.getCount() + 1);

into 3 separate lines, it'll be clearer:

final int oldCount = count.getCount(); // a
final int newCount = oldCount + 1; // b
count.setCount(newCount); // c

Note that while statements (a) and (c) are each synchronized, the entire block is not. So they can still interleave, which means thread A can enter/execute statement (a) after thread B executes thread (a) but before it finishes/enters statement (c). When this happens thread (a) and (b) will have the same oldCount and consequently misses one increment.

2.

join() is to make sure both thread A and Thread B finishes before you print. The reason you're getting a smaller count if because when you print your result thread A and B may not have finished running yet. In other words, even if you synchronize perfectly, without join(), you'll still have a number much smaller than 2000.

3. See answer to 2.

1) Because you're not locking correctly. You're calling getCount(), which locks, gets the value and unlocks, incrementing and calling setCount() which locks, saves the value and unlocks. What happens in some cases is that both threads call getCount(), the first tread locks, gets the value xand unlocks. Then the second thread gets the lock, gets the same value xand unlocks . Since both threads increment and later save the same value, you'll get a lower count than expected.

2) Without join() you won't wait for your threads to finish, your main thread will just call getCount() and get a random value while the threads are still running.

3) Since the other threads don't hold a lock all the time (if you want them both running, they need to give each other unlocked time after all), you'll need the join()still.

someThread.join() will cause the calling Thread to wait until someThread has completed.

If you remove the join() calls, then the main Thread may call getCount() before the count is finalized, because someThread may still be running.

A synchronized method simply means that more than one synchronized call to an Object can not be made at the same time.

The one line answer is that count.setCount(count.getCount() + 1); is not an atomic operation.

Or to be slightly less oracular, while setCount and getCount are properly thread-safe and atomic, there is nothing stopping another thread from calling either of those methods in between this thread's call to setCount and getCount. That will result in a count getting lost.

One way to avoid lost counts is to create an atomic increment() operation.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top