0

I have a counter and multiple threads access the getCount method. The code is like the following:

public class ThreadSafeMethod {

        public static int counter = 0;

        public static int getCount() {
            return counter++;
        }
    }

Is the method thread safe? My understanding is that because counter++ is not atomatic, it is not safe. Then how to make it safe? If we add synchronized keyword, what object will be synchronized?

Ryan
  • 2,825
  • 9
  • 36
  • 58

4 Answers4

5

You are correct in your analysis when you say that it's not thread safe because the operation is not atomic. The retrieval of the value and the increment are not thread safe. Multiple calls to this method (whether it has parameters or not) access the same non-local variable.

Adding synchronized to this method makes it thread-safe. When adding to a static method, then the Class object is the object which is locked.

An alternative to making it thread-safe is to replace the int with an AtomicInteger, which has its own atomic getAndIncrement method.

rgettman
  • 176,041
  • 30
  • 275
  • 357
3

No, a parameter-less method is not inherently thread-safe - the lack of parameters makes no difference in this example.

The read from (and write to) the counter variable is not guaranteed to be either atomic or consistent between threads.

The simplest change is to simply make the method synchronized:

public static synchronized int getCount() {
    return counter++;
}

(The simplest is not always the "best" or "correct", but it will be sufficient here, assuming that no other code touches the public counter variable.)

See this answer for how a synchronization of a static method works - as can be imagined, it is the Class itself that is used as the monitor.

Community
  • 1
  • 1
user2864740
  • 60,010
  • 15
  • 145
  • 220
1

Using the synchronised keyword on the static function will 'lock' the function to one thread at a time to ensure that two threads can not mess with the same variable. With what you propose, I believe anything that gets accessed or modified in that function will be thread safe.

Joey Clover
  • 756
  • 5
  • 14
  • 1
    "Anything that gets accessed or modified in that function will be thread safe." -- It's true that only one thread could enter getCount() in the example above at any given time if it was synchronized on the class object, but that alone would not make the count variable thread safe. What if some other method also directly accessed the count variable? The variable is only thread safe if _every_ method that touches it locks the _same lock_. – Solomon Slow Feb 27 '14 at 02:36
1

Just as you say the counter++ operation is non atomic so giving multiple threads access at once will result in undefined behaviour. In thread safety, the issue is almost always having synchronized access to shared resources such as static variables.

The lock which a thread acquires when declaring the method synchronized belongs to that class. Say we had two methods in a class

public synchronized void foo() {...}
public synchronized void bar() {...}

If one thread enters foo() it acquires the lock for the class, and any other thread trying to access foo() OR bar() will block until the first thread has finished. To overcome this, we can lock on seperate objects within the methods.

// Declare 2 objects to use as locks within the class
private Object fooLock = new Object();
private Objecy barLock = new Object();

// lock on these objects within the method bodies
public void foo {
    synchronized(fooLock) { /* do foo stuff */ }
}

public void bar() {
    synchronized(barLock) {/* do bar stuff */}
}

Now 2 threads can access the foo() and bar() simultaneously.

There's a lot of material on the web on Thread safety, I'd recommend this set of tutorials if yo want to know more about multithreading with locks / executor services and stuff.

StickyCube
  • 1,721
  • 2
  • 17
  • 22