Thread Safe Servlet With a Static String

Question

I have reviewed a sample of a chat server with Node JS and socket IO at http://ahoj.io/nodejs-and-websocket-simple-chat-tutorial. In that sample a simple history variable was used at server to save chat history data. As the Node Js is single thread every thing works fine. (You can ignore above node JS example if you are not interested in node js :) I will explain it in java below )

Consider below servlet which gets message String from request and add it to an string. This code could be an example of a Chat Server. It gets user messages from request and all it to a history String and other clients can read it.

public class ChatServlet implements Servlet {

    private static String history = "";    

    public void service(ServletRequest request, ServletResponse response)
         history = history.concat(request.getParameter("message"));
    }

}

Theoretically, this code is not thread-safe as it use a global static variable (How do servlets work? Instantiation, sessions, shared variables and multithreading) .

However, I have tested above code with jMeter with lots of concurrence request and the history string always stores all the messages (So no client message lost or over-written), and nothing went wrong! I have not work with threads so I wonder if I am missing something here ! Is the above code thread-safe and can it be trusted.

score 3 · Answer 1 · answered Jul 12 '15 at 10:31

As others have confirmed, this is indeed not thread-safe in that it cannot be trusted. Some quirk in JVM implementation may make this a workable servlet, but there is no guarantee that it will work at another JVM or even at another time.

To add to the variety of proposed implementations, here's one with AtomicReference:

AtomicReference<String> history = new AtomicReference<>("");

public void service(ServletRequest request, ServletResponse response)
     history.updateAndGet(h -> h.concat("123"));
}

score 1 · Answer 2 · answered Jul 12 '15 at 10:15

1

It isn't thread-safe. Code that isn't thread-safe isn't guaranteed to fail, but it's not guaranteed to work either.

answered Jul 12 '15 at 10:15

user1675642

747
2
5
15

score 1 · Accepted Answer · answered Jul 12 '15 at 10:16

No, it's not. Thread safety bugs can be difficult to trigger - maybe your program will miss one message in a billion, or maybe it will never miss a message by coincidence. If it was thread safe, though, it would be guaranteed to never happen.

You could simply use a synchronized block to ensure that only one thread accesses history at a time, like this:

synchronized(ChatServlet.class) {
    history = history.concat(request.getParameter("message"));
}

This means: lock ChatServlet.class, add the message to the history, then unlock ChatServlet.class.

You can never have two threads lock the same object at the same time - if they try, one of them will proceed, and the rest will wait around for the first one to unlock the object (and then another one will proceed, and the rest will wait for it to unlock the object, and so on).

Also make sure to only read history inside a synchronized(ChatServlet.class) block - otherwise, it's not guaranteed that the reading thread will see the latest updates.

Thread Safe Servlet With a Static String

3 Answers3