Usage of Static variables in Java Servlets (e.g. in AppEngine)

Question

I have an application where Servlet has a method called Update(ReqIn, ReqOut). I call it from doGet & doPost and pass the Request and Response variables, and it is then up to Update(...) to fill out the following static variables:

...
public class Server extends HttpServlet {

    public static HttpServletRequest In = null;
    public static HttpServletResponse Out = null;

    public static boolean isDebug = true;
    public static boolean isPost = false;

    public static String URL = "";
    public static String IP = "0.0.0.0";
    public static Cookie[] Cookies = null;

    public static UserClass User = null;
    public static boolean isLoggedIn = false;


    ...
}

Basically Abstracting most used stuff & updating it on every request. This also allows me to access IP address & current user data from anywhere in the website, by writting just Server.User.getUsername(); insead of making a new Class instance everytime a page is loaded and using much longer access code: Server.getUser().getUsername();

Now the question is: When in multi user environment (Jetty on AppEngine), can this introduce any problems? E.g. some threading/racing issues making user see incorrect IP address or in extreme case suddenly being logged in as different user?

Or should I rewrite code and change it to Public UserClass User instead of Public static UserClass User, etc?

Answer 1

Yes, this is a hugely bad idea!

What would you expect to happen if you got two requests at the same time? Each static variable can only hold one value, so you're going to lose data.

You could use ThreadLocal so that each thread only had access to the current request/user/etc that it was dealing with - but that's still basically a bad idea. It's brittle, and hides the fact that lower layers need the information. Pass the state down to the code that needs it instead.

Answer 2

Using statics is a hugely bad idea, since if you get two requests come in at the same time then they will write over each other. Take this trivial example to see what can go wrong:

1:public class Server extends HttpServlet {
2:  public static int requestNo = 0;
3:  public void doGet(HttpServletRequest req, HttpServletResponse resp)
4:  {
5:     requestNo++;
6:     resp.getWriter().println(requestNo);
7:  }
8:}

Now imagine the following timeline:

Request 1 comes in, and processes up to line 5.
Request 2 comes in, and processes completely.
Request 1 continues processing.

Both requests will get the text "2", instead of one getting "1" and one getting "2". This is a simple example of the state being stomped on.

Now, to answer the second part of your question;

Or should I rewrite code and change it to Public UserClass User instead of Public static UserClass User, etc?

No, that is also not good enough, since the J2EE spec allows the servlet container to use one instance of a class to service all of the requests for that servlet mapping, that is, the instance level variables will have exactly the same effect as being statics, they are shared between all requests.

This leaves only three real options:

1. Shove everything into HTTPSession. The problem here is that this is a map, so you lose type safety, and it is difficult to see where things are being used.
2. Create a Holder class to hold all of your state and pass that around everywhere. This is a little better, since at least you don't lose the type safety, but you still don't have full visibility either.
3. Pass the individual required items around.