CODEWITHSUNDEEP
javamultithreadingsynchronization
questborn
questborn

Reputation: 2855

Is a static counter thread safe in multithreaded application?

public class counting
{
  private static int counter = 0;

  public void boolean counterCheck(){
  counter++;
  if(counter==10)
  counter=0;
  }
}

Method counterCheck can be accessed by multiple threads in my application. I know that static variables are not thread safe. I would appreciate if someone can help me with example or give me reason why I have to synchronize method or block. What will happen if I don't synchronize?

Upvotes: 3

Views: 7060

Answers (6)

David Schwartz
David Schwartz

Reputation: 182819

It's clearly not thread-safe. Consider two threads that run in perfect parallel. If the counter is 9, they'll each increment the counter, resulting in the counter being 11. Neither of them will then see that counter equal to 10, so the counter will keep incrementing from then on rather than wrapping as intended.

Upvotes: 6

ratchet freak
ratchet freak

Reputation: 48216

first of counter++ by itself is NOT threadsafe

hardware limitations make it equivalent to 

int tmp = counter;
tmp=tmp+1;
counter=tmp;

and what happens when 2 threads are there at the same time? one update is lost that's what

you can make this thread safe with a atomicInteger and a CAS loop

private static AtomicInteger counter = new AtomicInteger(0);

public static boolean counterCheck(){
    do{
        int old = counter.get();
        int tmp = old+1;
        if(tmp==10)
            tmp=0;
        }
    }while(!counter.compareAndSet(old,tmp));
}

Upvotes: 0

Brendan Long
Brendan Long

Reputation: 54262

Imagine counter is 9.

Thread 1 does this:

counter++; // counter = 10

Thread 2 does this:

counter++; // counter = 11
if(counter==10) // oops

Now, you might think you can fix this with:

if(counter >= 10) counter -= 10;

But now, what happens if both threads check the condition and find that it's true, then both threads decrement counter by 10 (now your counter is negative).

Or at an even lower level, counter++ is actually three operations:

  • Get counter
  • Add one to counter
  • Store counter

So:

  1. Thread 1 gets counter
  2. Thread 2 gets counter
  3. Both threads add one to their counter
  4. Both threads store their counter

In this situation, you wanted counter to be incremented twice, but it only gets incremented once. You could imagine it as if this code was being executed:

c1 = counter;
c2 = counter;
c1 = c1 + 1;
c2 = c2 + 1;
counter = c1; // Note that this has no effect since the next statement overrides it
counter = c2;

So, you could wrap it in a synchronized block, but using an AtomicInteger would be better if you only have a few threads:

public class counting {
    private static AtomicInteger counter = new AtomicInteger(0);

    public static void counterCheck() {
        int value = counter.incrementAndGet();
        // Note: This could loop for a very long time if there's a lot of threads
        while(value >= 10 && !counter.compareAndSet(value, value - 10)) {
            value = counter.get();
        }
    }
}

Upvotes: 0

Zan Lynx
Zan Lynx

Reputation: 54345

This is not thread safe, AND this pattern of updating a count from multiple threads is probably the #1 way to achieve negative scaling (it runs slower when you add more threads) of a multi-threaded application.

If you add the necessary locking to make this thread safe then every thread will come to a complete halt while counting. Even if you use atomic operations to update the counter, you will end up bouncing the CPU cache line between every thread that updates the counter.

Now, this is not a problem if each thread operation takes a considerable amount of time before updating the counter. But if each operation is quick, the counter updates will serialize the operations, causing everything to slow down on all the threads.

Upvotes: 2

JB Nizet
JB Nizet

Reputation: 691913

It's NOT thread-safe, for multiple reasons. The most obvious one is that you could have two threads going from 9 to 11, as mentioned by other answers.

But since counter++ is not an atomic operation, you could also have two threads reading the same value and incrementing to the same value afterwards. (meaning that two calls in fact increment only by 1).

Or you could have one thread make several modifications, and the other always seeing 0 because due to the Java memory model the other thread might see a value cached in a register.

Good rule of thumb: each time some shared state is accessed by several threads, and one of them is susceptible to modify this shared state, all the accesses, even read-only accesses must be synchronized using the same lock.

Upvotes: 0

Dave Newton
Dave Newton

Reputation: 160251

Biggest danger? Two increments to counter before the counter == 10 check, making the reset to 0 never happen.

Upvotes: 0

Related Questions