-1

The answer should always be 500, but it keeps getting changed

I ran 1000 threads, if the thread is odd it should decrements the sum using a loop 100000 times, and if even increment 100001 times.

The answer keeps getting changed though i used the join method, any suggestions?

package softwaredesign.assignment1;

public class SoftwareDesignAssignment1 {

public static class  T implements Runnable{
    int e=0;
    T(int z){
        e=z;
    }
    public void run(){
       if(e==1){
       for(int i=0;i< 100001;i++)
       SoftwareDesignAssignment1.sum+=e;
       }
       else{
           for(int i=0;i< 100000;i++)
       SoftwareDesignAssignment1.sum=sum+e;
       }

    }


}
public static int sum=0;

public static void main(String[] args) throws InterruptedException {
    Thread[] t=new Thread[1000];

    for(int i=0;i<1000;i++){
        if(i%2==0)
        t[i]=new Thread(new T(1));
        else
        t[i]=new Thread(new T(-1));
    }
    for(Thread tt:t){
      tt.start();
    }

    for(Thread tt:t){
      tt.join();
   }
   System.out.print(sum);
}

}

Nathan Hughes
  • 94,330
  • 19
  • 181
  • 276
Ahmad Rashaideh
  • 7
  • 1
  • 7
    `+=` is [not a thread-safe operation](https://stackoverflow.com/questions/32676771/is-the-operator-thread-safe-in-java), for starters. – azurefrog Oct 23 '17 at 18:18
  • 2
    Search for "thread safety" and "thread atomic". This should also be covered in the source material, usually in the section that deals with `synchronized`. – user2864740 Oct 23 '17 at 18:18
  • @user2864740 Ok, thank you :) – davidxxx Oct 23 '17 at 18:20
  • The result of your code should be unpredictable because (as mentioned before) it is not thread safe. – DwB May 21 '18 at 01:57

1 Answers1

1

For a thread to update that value, it has to do the following:

  1. read the value from memory into a register
  2. add 1 to the value in the register
  3. store the updated value back to memory

Now, imagine that you have two threads that are trying to do that simultaneously. You start with the value in memory set to 0. Here is what can happen:

  1. Thread 1 reads the value 0 into a register
  2. Thread 1 updates the value in the register
  3. Thread 2 reads the value 0 into a register
  4. Thread 1 stores the value 1 into memory
  5. Thread 2 updates the value in the register
  6. Thread 2 stores the value 1 into memory

Both threads executed, but since they were both working on the same copy of the value, it's as though only one thread updated it.

Another thing that can happen is that the compiler can make the assumption that it has exclusive access to the value in memory. When it sees this code:

for (int i = 0; i < 1000; ++i)
{
    SoftwareDesignAssignment1.sum+=e;
}

The compiler optimizes the code by loading the value into memory once, incrementing it 1000 times, and then storing the value back to memory. There's no reason for it to do the "load, increment, store" 1000 times. If this happens with multiple threads in your program, the result will either be 1000 or -1000, depending on whether it was an increment or a decrement thread that finished last.

There are a number of ways around this problem that involve using primitives such as locks, interlocked directives, and language-specific synchronization operations. Links in the comments will help you with specifics in the Java language.

Jim Mischel
  • 131,090
  • 20
  • 188
  • 351