Count concurrent calls on a method

Question

Let's say I have a very simple web-service whose only task is to count how many times it's endpoint was called. The endpoint is /hello.

@Controller
public class HelloController {

    private int calls = 0;

    @RequestMapping("/hello")
    public String hello() {
        incrementCalls();
        return "hello";
    }

    private void incrementCalls() {
        calls++;
    }
}

Now this works all fine as long as two users don't call /hello at the same time, simultaneously. But when a parallel call to /hello does happen, the calls variable would only get incremented once (if I am not mistaken). So obviously some kind of synchronization would need to take place here.

The question is what would be the best way to make this method thread-safe?

Use `java.util.concurrent.atomic.AtomicInteger` instead of `int`. — Jesper, Nov 13 '17 at 15:12
Add an atomic long or integer for the call count and you are okay. Otherwise protect the call count variable by a lock. — Nipun Talukdar, Nov 13 '17 at 15:12
Related: https://stackoverflow.com/questions/16795303/must-spring-mvc-classes-be-thread-safe — assylias, Nov 13 '17 at 15:13

Mad Physicist · Accepted Answer · 2017-11-13T17:36:53.747

The reason that calls++ could cause a behavior other than you expect is if it is not atomic. Atomic operations happen in such a way that the entire operation can not be intercepted by another thread. Atomicity is implemented either by locking the operation, or by harnessing hardware that already performs it an atomic manner.

Incrementing is most likely not an atomic operation, as you have supposed, since it is a shortcut for calls = calls + 1;. It could indeed happen that two threads retrieve the same value for calls before either has a chance to increment. Both would then store the same value back, instead of one getting the already-incremented value.

There are a few simple ways of turning the get-and-increment into an atomic operation. The simplest one for you, not requiring any imports, is to make your method synchronized:

private void incrementCalls() {
    calls++;
}

This will implicitly lock on the HelloController object it belongs to whenever a thread enters the method. Other threads will have to wait until the lock is released to enter the method, making the whole method into an atomic operation.

Another method would be to explicitly synchronize the portion of the code that you want. This is often a better choice for large methods that do not need to have a lot of atomic operations, since synchronization is fairly time- and space-expensive:

private void incrementCalls() {
    sychronized(this) {
        calls++;
    }
}

Making the whole method synchronized is just a shortcut for wrapping its entire contents in synchronized(this).

java.util.concurrent.atomic.AtomicInteger handles the synchronization to make most of the operations you would want to do on an integer into atomic operations for you. In this case you could call getAndAdd(1), or getAndIncrement(). This would probably be the cleanest solution in terms of keeping the code legible as it reduces the number of braces and uses carefully designed library functions.

Count concurrent calls on a method

1 Answers1