Servlets and synchronization

Question

Servlets runs in several threads, so my question is:

If I have a lot of servlets which call some utility class (DbUtils, for example

Connection c = DbUtils.getConnection();
//....some action with db here

should I assume additional actions for synchronization inside DbUtils?

Actually I'd like to inherit HttpServlet into something like DatabaseInvokerServlet:

public abstract class DatabaseInvokerServlet extends HttpServlet

with methods:

public abstract void getResultSets(Connection connection) throws SQLException;
private AbstractUser currentUser;
private HttpServletRequest request;
private HttpServletResponse response;
protected void processData() {}
protected void afterRequestProcessed() throws ServletException, IOException {}
protected void beforeRequestProcessed() throws ServletException, IOException {}

protected void execute() {
    Connection c = null;
    try {
        c = DbUtils.getConnection();
        getResultSets(c);
        processData();
    } catch (SQLException e) {
        e.printStackTrace();
    } finally {
        try {
            if (c != null) {
                c.close();
            }
        } catch (SQLException e) {
            e.printStackTrace();
        }
    }
}

public HttpServletRequest getRequest() {
    return request;
}

public HttpServletResponse getResponse() {
    return response;
}

public AbstractUser getCurrentUser() {
    return currentUser;
}

protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
    request.setCharacterEncoding("UTF-8");
    response.setContentType("text/html;charset=UTF-8");
    response.setCharacterEncoding("UTF-8");


    this.request = request;
    this.response = response;
    this.currentUser = (AbstractUser) request.getSession().getAttribute("currentUser");

}

Then I'd just inherit my DatabaseInvokerServlet to new servlets to do custom stuff. The reason is not to copy-paste database invoke block with try-catch-finally in a lot of places.

But as I can see such approach won't work because of synchronization issues. Am I right?

Please post the code of DbUtils.getConnection(); – Hendrik Brummermann Mar 18 '11 at 07:22 — Hendrik Brummermann, Mar 18 '11 at 07:22

score 3 · Accepted Answer · edited Jun 20 '20 at 09:12

If the DbUtils creates the connection in the same thread, like as:

public static Connection getConnection() throws SQLException {
    return DriverManager.getConnection(url, username, password);
}

Then it's threadsafe.

But if the connection is a class variable, like as:

private static Connection connection = DriverManager.getConnection(url, username, password);

public static Connection getConnection() throws SQLException {
    return connection;
}

Then it is definitely not threadsafe because the same connection will be shared among all threads. Also when it's closed in a thread, all subsequent threads won't be able to use the connection because it's not open anymore. Also when it's never closed, the DB will timeout the connection sooner or later, usually after a few hours, and your application won't work anymore because the connection is not open anymore.

As to the servlet,

public abstract class DatabaseInvokerServlet extends HttpServlet {
    private AbstractUser currentUser;
    private HttpServletRequest request;
    private HttpServletResponse response;
    // ...
}

it's definitely not threadsafe. You're assigning the current user, request and response as instance variables. From each servlet class, there is only one instance during the application's lifetime. This instance is shared among all visitors/sessions throughout the entire application's lifetime. Each HTTP request operates in a separate thread and uses the same instance.

Imagine two simultaneous visitors: visitor A will set the current user, request and response. The DB process however takes a long time. Before the response of visitor A has returned, visitor B calls the same servlet and thus the current user, request and response will be overriden. Then, the query of visitor A finishes and wants to write to the response, it is instead writing to the response of visitor B! Visitor B sees the result of the query of visitor A and visitor A sees nothing on his screen!

You should never assign request/session-specific data as instance variable of the servlet. You should keep them method (thread) local.

public abstract class DatabaseInvokerServlet extends HttpServlet {
    protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
        AbstractUser currentUser = request.getSession().getAttribute("user");
        // Keep the variables in the method block! 
        // Do not assign them as instance variable!
    }
}

As to the complete picture, this approach is clumsy. The database access layer should have nothing to do with servlets. It should operate in its own standalone classes which you could just construct/invoke in every other Java class, any servlet class, or a normal application with main(), or whatever. You should not have any single line of java.sql.* imports in your servlet classes (expect of maybe SQLException if it is not abstracted away). You should not have any single line of javax.servlet.* imports in your database classes.

Servlets and synchronization

4 Answers4

See also: