1

So here I write three simple class to inspect how multiple threads working in java, but they produce different result everytime I run it. Here is the code:

        public class Accum {

        private static Accum a = new Accum();
        private int counter = 0;

        private Accum(){}

        public static Accum getAccum(){
            return a;
        }

        public void updateCounter(int add){
            counter+=add;
        }

        public int getCount(){
            return counter;
        }
    }//end of class

        public class ThreadOne implements Runnable {
        Accum a = Accum.getAccum();
        public void run() {
            for(int x=0; x<98; x++){
                //System.out.println("Counter in TWO "+a.getCount());
                a.updateCounter(1000);
                try{
                    Thread.sleep(50);
                }catch(InterruptedException ex){}
            }
            System.out.println("one " + a.getCount());
        }
    }//end of class

        public class ThreadTwo implements Runnable{
        Accum a = Accum.getAccum();
        public void run() {
            for(int x=0; x<99; x++){
                //System.out.println("counter in Two "+a.getCount());
                a.updateCounter(1);
                try{
                    Thread.sleep(50);
                }catch(InterruptedException ex){}
            }
            System.out.println("two "+a.getCount());
    }

        public class TestThreaad {
        public static void main(String[]args){
            ThreadOne t1 = new ThreadOne();
            ThreadTwo t2 = new ThreadTwo();

            Thread one = new Thread(t1);
            Thread two = new Thread(t2);

            one.start();
            two.start();
        }
    }end of class

So the expected result would be : one 98098, two 98099, but it turns out that the results are just unpredictable, sometimes it would be 78000 or 81000, I don't know..

but if i add some code to print a line of current value of count, the final result would be correct..

I really have no idea what is going wrong, and even add the keyword synchronized in the ThreadOne and ThreadTwo, run() method, the problem is still there...

I've studied the java for 3 months and this is the most elusive problem I've ever confronted...so thanks in advance for anyone could help me to understand the basic point of multiple threading...

Crazy Mulberry
  • 127
  • 1
  • 2
  • 8

3 Answers3

2

Code is not synchronized. As it is unsynchonized different Thread trying to update counter might be at same time which cause this problem.

If you synchonized updateCounter then access of this method will be in proper.

public synchronized void updateCounter(int add){
      counter+=add;
}
Subhrajyoti Majumder
  • 40,646
  • 13
  • 77
  • 103
  • Thanks a lot, the book I'm reading is Head First Java 2dn Edition, and it says that if I write such clause like i++, i+=100, it will be consider as atomic... – Crazy Mulberry Sep 02 '13 at 11:37
  • there are `java.util.concurrent.atomic.AtomicInteger` avaiable in java – Subhrajyoti Majumder Sep 02 '13 at 11:39
  • @user2548760, I believe the book is wrong then. See http://stackoverflow.com/questions/15287419/is-i-atomic-in-java for instance. The JLS is probably the definitive source. The operation is atomic only in the sense that the whole result is stored at once, not in the sense that the operation is performed safely with respect to actions of other threads. – davmac Sep 02 '13 at 11:54
  • Throw that book away. Read http://docs.oracle.com/javase/tutorial/essential/concurrency/interfere.html and the rest of that tutorial. – Ralf H Sep 02 '13 at 13:10
1

In your example, the Accum instance is shared between the threads. Your update process is a typical READ, COMPUTE-UPDATE, WRITE operation sequence. Because the resource is shared and not protected, those three operations (from two threads -- making six operations) can be interleaved in many different ways, causing updates to be lost.

Here is an example ordering of the operations (number indicates thread):

READ #1           -> reads 10
COMPUTE-UPDATE #1 -> computes 1010
READ #2           -> reads 10
WRITE #1          -> writes 1010
COMPUTE-UPDATE #2 -> computes 11
WRITE #2          -> writes 11 (earlier update is lost)

So you see, almost any result is possible. As @SubhrajyotiMajumder notes, you can use synchronized to fix it. But if you do that, maybe threads aren't right for your problem; or alternatively, you need another algorithmic process.

Kevin A. Naudé
  • 3,992
  • 19
  • 20
0

Your code is not synchronized properly. As an alternative to a synchronized method I would suggest using AtomicInteger to store the variable accessed and modified from different threads.

Dariusz
  • 21,561
  • 9
  • 74
  • 114