Increment an int using threads

Question

I have two threads and I want to increment an int variable while it is less than a certain value. My program does that but two different threads do not increment the value separately.
If one and two are two threads, I want the output to be like this

one 1  
two 2  
one 3  
two 4  
one 5  
two 6  
one 7  
two 8...  

Here is my program:

class IncTo100Demo implements Runnable {
    public volatile int count = 0;

    public void run() {
        Thread current = Thread.currentThread();
        try {
            while(count < 21) {
                System.out.println(current.getName() + " count = " + count++);
                current.sleep(1000);
            }
        }
        catch(Exception e) {
            e.printStackTrace();
        }
    }

    public static void main(String[] args) {
        Thread one = new Thread(new IncTo100Demo(), "one");
        Thread two = new Thread(new IncTo100Demo(), "two");

        try {
            one.start();
            two.start();
            System.out.println("one alive? " + one.isAlive());
            one.join();
        }
        catch(Exception e) {
            e.printStackTrace();
        }
    }
}  

and its output is:

one alive? true
two count = 0
one count = 0
two count = 1
one count = 1
two count = 2
one count = 2
two count = 3
one count = 3
two count = 4
one count = 4
two count = 5
one count = 5
two count = 6
one count = 6
two count = 7
one count = 7
two count = 8
one count = 8
two count = 9  

How can I make these threads update the variable individually so that the output can look like what I want it to?

Solution

count is an instance variable of class IncTo100Demo. Each of your threads uses its own instance of that class, and therefore works with its own copy of that variable. That's why they update the values independently. To make the threads share, you must either have them use the same instance of your class, or you must make count static.

Furthermore, once the two threads are accessing the same count variable, having that variable be volatile is not sufficient to guarantee the behavior you want. You are fairly likely to see that behavior, but it would still be possible for one thread to fail to see the other's updates to the variable. This is because evaluating the expression count++ and applying its side effect is not atomic. It is possible, therefore, that both threads read the same value of count (i.e. before either one writes an updated value). In that case, both will compute the same updated value, and both will write it. The result would look similar to the erroneous output you present, though it would be likely to be (very) occasional, not consistent.

To avoid that problem, you need to synchronize access to count (which then does not need also to be volatile). You cannot do that by synchronizing the run() method, however, for that would make the method synchronize using the monitor of the object on which run() is invoked. If your threads use the same IncTo100Demo instance then that will prevent any concurrency, and if they do not then that will not be effective.

Instead, use a synchronized block inside run(). If the two threads share the same IncTo100Demo instance (which I recommend) then you can synchronize on that instance:

synchronized (this) {
    // ... read and manipulate 'count'
}

Otherwise, you'll need to create another object shared between the the threads, and synchronize on that. Be careful that all accesses to count on each loop iteration are within the same synchronized block (including the count < 21 test), but that the sleep() is outside it. You want the scope of the synchronization to be as narrow as possible.

Since this appears to be an educational exercise, I leave the details for you to work out, but I'll give you this hint: you probably need to re-structure your loop.

Comments

[strider14]

Thanks for the explanation John. Does it mean the we should always use synchronize when dealing with threads? This is just a practice problem I came across. I am trying to learn more about threads and your info is quite helpful. I used static and it worked.

[John Bollinger]

If you share data between threads then you must properly synchronize access to that shared data. Under some circumstances it is enough to declare the data volatile, or to use atomic data types, or other classes from java.util.concurrent, but sometimes directly synchronizing is indeed the best, and perhaps even the only reasonable way to go.