0

I am trying to implement a counter using atomic integer and then printing the final value after all threads finish their tasks , but i am not getting the latest value , in my case the value i should get should be 20 but it is fluctuating like 17,18,16,19,20 etc

the program is as follows:

class AtomicOperations implements Runnable 
{
private AtomicInteger ai;
private int a;

AtomicOperations(int aivalue, int ivalue) {
    ai = new AtomicInteger(aivalue);
    this.a = ivalue;
}

@Override
public void run() {
    ai.getAndIncrement();
}

  public static void main(String args[]) {
    AtomicOperations obj = new AtomicOperations(10, 10);
    Thread[] t = new Thread[10];
    synchronized (obj) {
        for (int i = 0; i < t.length; i++) {
            t[i] = new Thread(obj, "Thread-" + (i + 1));
            t[i].start();
        }
    }
    System.out.println(obj.ai);
  }
}
Amol
  • 303
  • 4
  • 13
  • 1
    What do you think your `synchronized(obj)` is good for? – Kayaman Oct 03 '16 at 10:10
  • Atomic does not need synchronization. – Noor Nawaz Oct 03 '16 at 10:12
  • @Kayaman .. i thought this may complete all threads before the print statement runs – Amol Oct 03 '16 at 10:13
  • 1
    Possible duplicate of [Wait until child threads completed : Java](http://stackoverflow.com/questions/9939076/wait-until-child-threads-completed-java) – user140547 Oct 03 '16 at 10:13
  • @Amol It doesn't. It does nothing of that sort. In fact in your code it does absolutely nothing. You have no chance of writing correct multithreaded code if you're guessing things. – Kayaman Oct 03 '16 at 10:14

1 Answers1

0

You aren't waiting until the threads are done, so the number of increments that have completed when you print can vary.

David Schwartz
  • 179,497
  • 17
  • 214
  • 278
  • i added a synchronized block .. i thought it my do – Amol Oct 03 '16 at 10:11
  • 1
    You will *never* get synchronization to work if you sprinkle things around without understanding *precisely* what synchronization you need. Here, what you need is some way to know when the threads are finished. A `synchronized` block doesn't wait for something to finish, it just doesn't interrupt it in the middle. (More or less.) Although here it does nothing at all since it doesn't protect anything that's shared. – David Schwartz Oct 03 '16 at 10:12
  • point taken ... main execution will surpass sync block ..so i need to use join() ? – Amol Oct 03 '16 at 10:19
  • @Amol That would be one option. Personally, I don't like using `join` but in this case, it happens to do exactly what you need so it's probably the simplest solution. (I don't like using `join` because what you really want to wait for is the work to be finished, not the threads to be finished. So while it works, it's inelegant and makes the design fragile since you can't later make the threads do other stuff too without breaking the synchronization.) – David Schwartz Oct 03 '16 at 10:25