Threads and shared counters in java

I got this exercise at uni:

Write a program that declares a shared integer counter and then creates two threads, one of which attempts to increment the counter 1000 times and the other attempts to decrement the counter 1000 times. When each thread has finished looping, it should print out the final value of the counter. (Hint: You will need to define a Counter class. Why?) What do you think the output should be? Did the program work as you expected? Try running the program repeatedly to see if you always get the same result.

I ran the program expecting the final result to be zero, but it actually outputs a different number between 0 and 1000 each time. Can anyone tell me why? Thanks.

public class Counter
{
    private int val;

    public Counter()
    {
        val = 0;
    }

    public void increment()
    {
        val = val + 1;
    }

    public void decrement()
    {
        val = val - 1;
    }

    public int getVal()
    {
        return val;
    }
}

public class IncThread extends Thread
{
    private static final int MAX = 1000;
    private Counter myCounter;

    public IncThread(Counter c)
    {
        myCounter = c;
    }   

    public void run()
    {
        for (int i = 0; i < MAX; i++)
        {
            myCounter.increment();
        }
    }

}

public class DecThread extends Thread
{
    private static final int MAX = 1000;
    private Counter myCounter;

    public DecThread(Counter c)
    {
        myCounter = c;
    }

    public void run()
    {
        for (int i = 0; i < MAX; i++)
        {
            myCounter.decrement();
        }
    }
}

public class Main
{
    public static void main(String[] args)
    {
        Counter c = new Counter();

        Thread inc = new IncThread(c);
        Thread dec = new DecThread(c);

        inc.start();
        dec.start();

  开发者_JAVA百科      System.out.println(c.getVal());

    }
}

At the lowest level, your code probably boils down to something like:

Thread1                Thread2
-------                -------
do 1000 times          do 1000 times          
    get reg from [a]       get reg from [a]   
    reg = reg + 1          reg = reg - 1
    store reg to [a]       store reg to [a]

Because of that and the fact that threads can be stopped and started at any point of their execution, you have the possibility of this:

Thread1                Thread2
-------                -------
get reg from [a] (0)
                       get reg from [a] (0)
reg = reg + 1 (1)
                       reg = reg - 1 (-1)
                       store reg to [a] (-1)
store reg to [a] (1)

You can see that, although both threads have finished exactly one of the thousand cycles and you expect the count to be zero, it is actually one.

When sharing variables among threads of execution, you need to ensure that reads, writes and updates are atomic (there are exceptions but they're rare).

To do this, you should look into the various operations provided in your environment for just this purpose (such as synchronisation features in the language, mutexes (mutual exclusion semaphores) or atomic variables.

Look into why multi threaded programs use synchronization and you should find both the reason why this is a problem and also how to fix it.

Think about concurrency like this:

Suppose I have a point in memory called x.
I have a function called addOne which takes the value of x increments it by 1 and writes that value to x.
I have another function called subOne which takes the value at x decrements it by 1 and writes that value to x.

What if x = 10. addOne is called and grabs that value of 10 and begins processing it, but before it writes the value, subOne is called. subOne is going to grab x as 10, though if we were thinking about this linearly the value should be 11. Depending on which function writes last, the value will be different.

This should help you on the way to getting your answer, but this is something that you're going to have to think about yourself. That is the point of homework: to learn.

I don't want to spoil it for you, but consider what would happen if you turned your main method into this:

Before you run it... THINK about it! :)

public static void main(String[] args)
    {
        Counter c = new Counter();

        Thread inc = new IncThread(c);
        Thread dec = new DecThread(c);

        inc.start();
        dec.start();

        System.out.println(c.getVal());
        try { Thread.sleep(1000); } catch(Exception e) { }
        System.out.println(c.getVal());

    }

EDIT: Pax mentions something important. It's not the same issue that my code example describes, but it's still important nevertheless.

By the way, here's the output when I do that (The last run illustrates Pax's problem):

C:\Documents and Settings\<redacted>\My Documents>java Main
1000
0

C:\Documents and Settings\<redacted>\My Documents>java Main
82
82

C:\Documents and Settings\<redacted>\My Documents>java Main
1000
0

C:\Documents and Settings\<redacted>\My Documents>java Main
0
5