Thread safe counter- Atomic Integer not working

Question

I have (say) 10 threads. All threads increment the counter and the same will be logged. The counter is an atomic integer but sometimes I am getting duplicate values in the counter. What I expect is- counter to be threadsafe, allow counter to be incremented by an only thread at a time, read by an only thread at a time. What should I do differently?! Thank you

Class A{
 public void AM(){
   ExecutorService executor = Executors.newFixedThreadPool(threadPoolCount);
   List tasks = new ArrayList(tasklist);
   for(loop tasks) {
   Future<t> futureObjects = executor.invokeAll(task);
  }
 }
}
Class B implements Callable{
    public Object call(){
      //call method log
  }
}
Class C {
  static AtomicInteger counter = new AtomicInteger(1);
  public void log(){
    log("counter: "+counter)
    counter.getAndIncrement();
  }
}

note that `log("counter: "+counter); counter.getAndIncrement();` is different from `log("counter:"+counter.getAndIncrement());` — jhamon, Mar 12 '20 at 09:38
@QBrute Yes issue is still there, I made a mistake here while trying to change the variable names — grootygroot, Mar 12 '20 at 09:39
@jhamon is correct, printing counter is not guaranteed to be correct as the threads could be swapped between that print and the next statement. — mwarren, Mar 12 '20 at 09:40
@jhamon I have tried both variations. I get the same result. Threads are reading the counter and incrementing it at a same time. I guess. — grootygroot, Mar 12 '20 at 09:41

score 3 · Accepted Answer · answered Mar 12 '20 at 09:41

3

With given below code, you wont see duplicate values in your log. You are trying to print before updating which could lead to multiple thread printing the same value:

Class C {
  static AtomicInteger counter = new AtomicInteger(1);
  public void log(){
    int count = counter.getAndIncrement();
    log("counter: "+count)
  }
}

answered Mar 12 '20 at 09:41

Tarun

3,162
3
29
45

4

or remove the `count` variable and directly call `getAndIncrement` in the `log`parameter – jhamon Mar 12 '20 at 09:42
@jhamon Absolutely, I just wanted to make things easier to explain. – Tarun Mar 12 '20 at 09:43
@Tarun Edit: Thank you, I tried your solution - It is working now. Can you please explain why assigning it to local int works?! – grootygroot Mar 12 '20 at 10:23
Sorry for the confusion in my earlier(now deleted) comment. It was still pointing to old code hence didn't work. – grootygroot Mar 12 '20 at 10:25
@grootygroot Are you getting duplicate values in your log after trying above solution? – Tarun Mar 12 '20 at 10:27
@Tarun, No I am not getting duplicates now. Can you please explain how it works?! – grootygroot Mar 12 '20 at 10:28
1

@grootygroot You were printing the value before incrementing it. When multiple threads try to print at the same time, they all read the same value and print the same value. In above example, if multiple threads try to update the value at the same, they all would get a different updated value (because we are using atomic integer) in count variable. – Tarun Mar 12 '20 at 10:37

Thread safe counter- Atomic Integer not working

1 Answers1