Hi, I am developing a java servlet that while running, starts different objects methods in new threads. Those threads should access a variable that describes the specific servlet instance, say jobId. For this reason, i declared the jobId variable as static. The servlet constructor is calculating this value for each servlet instance (call). I was wandering if the servlet is called few times at the same time, the static jobId variable is shared between the calls, which means that some threads will get the wrong jobId, or it is calculated once for each call- so the threads that a specific servlet started will use the jobId calculated for this specific servlet (which is the way i want it to work). Any ideas? Thanks a lot!
static
means that every instance will access to the same value.
So every user connected to the servlet will access to the same value. Your jobId will be probably wrong when 2 users or more are connected together.
You have to get your own value a each connection and store it somewhere else.
Resources :
On the same topic :
Static variables are shared. Static variables don't belong to any one instance, they are accessible by all instances of the class. When you are using a constructor, that is used to create one object (one instance of the class), setting a static variable in a constructor typically doesn't make sense because it's something that is outside the scope of the object you're creating.
As for what would work, you could put the jobId in the HttpSession and then each user would have their own copy of it.
A servlet is created only once on webapp's startup and shared among all requests. Static or not, every class/instance variable is going to be shared among all requests/sessions. You would not like to assign request/session scoped data to them. Rather declare/assign them as methodlocal variable. E.g.
public class MyServlet extends HttpServlet {
private static Object thisIsNotThreadsafe;
private Object thisIsAlsoNotThreadsafe;
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
Object thisIsThreadsafe;
thisIsNotThreadsafe = request.getParameter("foo"); // BAD! Shared among all requests.
thisIsAlsoNotThreadsafe = request.getParameter("foo"); // BAD! Shared among all requests.
thisIsThreadsafe = request.getParameter("foo"); // Good.
}
}
There exist the legacy and deprecated SingleThreadModel
interface which you can let your servlet implement to force creation during every request. But this is a bad design and unnecessarily expensive. That's also why it's deprecated.
See also:
The instantiation policy for servlets is not defined in the servlet spec (as far as I can remember, anywho) but the usual behavior seems to be to create only one instance per servlet configuration. So, in your case, every request would use the same variable.
If I were you, I'd consider to send the jobId as a parameter to the Runnable
s you're running the threads with. For example, in stead of this:
public class HelloWorld extends HttpServlet {
private static long jobId;
public void doGet(HttpServletRequest request, HttpServletResponse response)
throws ServletException, IOException {
jobId = Long.parseLong(request.getParameter("jobid");
new Thread(new Worker()).start();
}
static class Worker implements Runnable {
@Override
public void run() {
doSomethingWith(jobId);
}
}
}
Refactor away the static variables like this:
public class HelloWorld extends HttpServlet {
// private static long jobId; -- delete, no longer needed
public void doGet(HttpServletRequest request, HttpServletResponse response)
throws ServletException, IOException {
long jobId = Long.parseLong(request.getParameter("jobid"); // local variable
new Thread(new Worker(jobId)).start(); // send jobId as parameter
}
static class Worker implements Runnable {
private final long jobId; // non-static; every instance has one
public Worker(long jobId) { // allow injection of jobId
this.jobId = jobId;
}
@Override
public void run() {
doSomethingWith(jobId); // use instance variable instead of static
}
}
}
Easier to read, no concurrency problems - pure win.