Servlet issue when high concurrency happens

Question

This is a sister question related to my previous post here: Simple web server when high-concurrency is met

The interview question is:

public class CounterServlet extends HttpServlet{

 private volatile static int counter=0;

 public int getCounter()
 {
   return counter;
 }

 public void service(HttpServletRequest request
                     HttpServletResponse response) throws IOException
 {
    counter++;
    PrintWriter out=response.getWriter();
    out.write("hello");
 }

What issue will there be in the code above when high concurrency is encountered? My analysis is:Servlet is singleton, thus there will be issues in synchronization.It has declared the counter as volatile, which will not prevent the issue.I suggest synchronize the service method?

Answer 1

If you access a static values through multiple thread each thread can have it's local cached copy! To avoid this you can declare the variable as static volatile and this will force the thread to read each time the global value. However, volatile is not a substitute for proper synchronisation!

You need to synchronize the code, although you are simply doing an increment on the counter but that does not mean that the entire method will be atomic. There may be multiple threads incrementing it at the same time using the current value from the registers. It may lead to undesirable result.

You need to either synchronize the method or use the AtomicInteger for such a trivial operation.

Answer 2

As volatile is just telling compiler not to optimize this variable, it won't help the issues related to concurrency.

I don't know what you are going to do with counter as you are only incrementing it but we might be sure, that after N calls of the service method, the counter will not be equal to N.

To prevent it, one might either make the method synchronized (which I assume is not the right approach), synchronize the incrementing part on some lock object or (I think is the most proper way to do it) use AtomicInteger instead of int - AtomicInteger class ensures that all operations on the objects are done atomically.

Answer 3

You should use AtomicInteger for that purpose.But since only one instance of the Servlet exists which is reused for multiple requests from multiple clients.Hence don't declare any instance or class variable in a Servlet and don't make methods synchronized.You can do something like below, if you want to use primitive int.

public void service(HttpServletRequest request
                 HttpServletResponse response) throws IOException
{
   synchronized (CounterServlet.class) {
        count++;
    }      
   PrintWriter out=response.getWriter();
   out.write("hello");
}