commons-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ben Walding <...@walding.com>
Subject Re: DBCP "Statement already closed .... how?"
Date Tue, 25 Nov 2003 08:24:57 GMT
I suspect a lot of your problems are due to code duplication.  How so?

You have the same boilerplate code for closing the connection in several 
places - extract it into a method.

Re: this code.

Context ctx = new InitialContext();
      if (ctx == null) {
        setError(CONTEXT_ERR);
        err();
        return;
      }

I haven't closely read the JLS, but I doubt this could even happen.  The constructor either
succeeds or throws an exception.


while (conn == null || conn.isClosed()) {
          conn = ds.getConnection();
}

This will thrash your machine while it tries to acquire a connection if the DB is down, at
least put a wait state in.



    session = request.getSession(true);
    if (session == null) {
      setErr(SESSION_ERR);
      err();
      return;
    } else {
      username = request.getRemoteUser();
    }

According to the spec, request.getSession() / request.getSession(true) will create a session
if none exists.  Hence my reading suggests that session will never be null for your if test.
 




Of course this doesn't really answer your question at hand - why are my connections falling
apart.  I suspect that if you cleaned things up and made it more modular, you would start
to see where your connections would be getting used.  I suspect that you are probably throwing
away a connection in some other code - eg Client, but not realising this inside your servlet.
This will typically be caused by unclear responsibilities of components. eg. Why does Client
need a connection?  would they be better off with a datasource that they can acquire a connection
from and destroy themselves?   

Since there is virtually no JDBC code in this example I assume that most of it occurs in LookupGeneral
and DoLookup etc, the code for those is probably of more interest.

IllegalStateExceptions typically occur if you acquire the PrintWriter 
and then try to do a sendRedirect  or something along those lines, 
because your doGet method is one big if-then-else nest, it is really 
hard to work out where this might be occurring.  Better to split out the 
actions out into methods where you can properly see if they doing bad 
things one at a time.


Cheers,

Ben


Anthony Presley wrote:

>On Mon, 2003-11-24 at 18:18, Vic Cekvenich wrote:
>  
>
>>Consider testing the app before deploying, such as OpenSTA.
>>    
>>
>
>Agreed.  It was tested "relatively" extensively for about six months
>amongst the various offices, with no problems.  Problems occur only when
>everyone is on it.
>
>  
>
>>What DAO layer are you using?
>>(ibatis, hibrenate, ?)
>>    
>>
>
>Not using one, using JDBC directly through the DBCP commons.  Using one
>would require retooling the 100K+ lines of code.
>
>Following is some example code, again, it works fine with one request to
>the servlet (say, I click and load one client).  But, if I click on more
>than one client (say, six one after another), I start getting Error
>500's, and the logs read "Connection closed" or "Statement already
>closed".  This appears to happen (from what I can tell) most often in
>the JSP.  My theory, currently, is:
>
>	Servlet is called / created
>	JDBC calls made / JSP called (via forward)
>	Connection is closed
>	Return from forward to JSP
>	Finally{} from try block is called, connection is not null, but not
>open, error is thrown.
>
>Of course, unless the app is grabbing the SAME connection, I don't see
>why this would be happening (no JDBC calls occur in the forwards).
>
>On the other hand, closing the connection prior to the forward() doesn't
>give the above errors, but does give the error:
>
>java.lang.IllegalStateException
>        at
>org.apache.coyote.tomcat4.CoyoteResponseFacade.sendRedirect(CoyoteResponseFacade.java:340)
>
>What does that mean <Google's not helping too much>?
>
>Some example code:
>
>  protected void processRequest(HttpServletRequest request,
>HttpServletResponse response)
>  throws ServletException, java.io.IOException {
>    // Create / validate the current session
>    session = request.getSession(true);
>    if (session == null) {
>      setErr(SESSION_ERR);
>      err();
>      return;
>    } else {
>      username = request.getRemoteUser();
>    }
>    
>    try {
>      javax.servlet.ServletContext context = getServletContext();
>      String dbInit = context.getInitParameter("dbInit");
>      if (dbInit == null) {
>        setError(DBINIT_ERR);
>        err();
>        return;
>      }
>      
>      Context ctx = new InitialContext();
>      if (ctx == null) {
>        setError(CONTEXT_ERR);
>        err();
>        return;
>      }
>      
>      DataSource ds = (DataSource) ctx.lookup(dbInit);
>      if (ds != null) {
>        conn = ds.getConnection();
>      } else {
>        setError(CONTEXT_ERR);
>        err();
>        return;
>      }
>      
>      if (conn == null || conn.isClosed()) {
>        // Try again to fetch a new connection?
>        log("Connection was closed");
>        while (conn == null || conn.isClosed()) {
>          conn = ds.getConnection();
>        }
>        log("Reopened the connection");
>      }
>      
>      // Did not make a valid connection for some reason, take to error
>screen
>      if (conn == null) {
>        setError(DBNULL_ERR);
>        err();
>        return;
>      } else {
>        // Should be a valid user
>        if (username == null) {
>          setError(NOUSER_ERR);
>          err();
>          try {
>            if (conn != null) {
>              conn.close();
>              conn = null;
>            }
>          } catch (SQLException e) { ; }
>          return;
>        }
>      }
>      
>      log("Sure of connection ... should NEVER be closed.");
>    } catch (Exception e) {
>      setError(CONTEXT_ERR);
>      err();
>      try {
>        if (conn != null) {
>          conn.close();
>          conn = null;
>        }
>      } catch (SQLException ex) { ; }
>      return;
>    }
>    
>    // Valid user, valid roles
>    // Process their data
>    String action = (String) request.getParameter("action");
>    String type = (String) request.getParameter("type");
>    if (action == null) {
>      // No action to perform, error
>      setError(ACTION_ERR);
>      err();
>      try {
>        if (conn != null) {
>          conn.close();
>          conn = null;
>        }
>      } catch (SQLException e) { ; }
>    } else if (action.equalsIgnoreCase("search")) {
>      // LOOKING for a client or lead
>      // Search based on lname, fname AND/OR unumber
>      
>      String lname = request.getParameter("lname");
>      String fname = request.getParameter("fname");
>      String unum = request.getParameter("clientnr");
>      try {
>        DoSearch(request);
>      } catch (Exception e) {
>        setError("Error searching clients for: " + username);
>        setExtraErr(e.getMessage());
>        err();
>      } finally {
>        try {
>          log("Closing");
>          if (conn != null) {
>            conn.close();
>            conn = null;
>          }
>          log("Closed");
>        } catch (SQLException e) { ; }
>      }
>    } else if (action.equalsIgnoreCase("view")) {
>      try {
>        Client c = new Client(conn, true);
>        
>        String cname = request.getParameter("clientnr");
>        log("Looking up username: " + username);
>        
>        if (!c.queryExactUserName(cname)) {
>          // Should be an error, but instead, set as a message...
>          req.setAttribute("msg", "Cannot find client with client number
>" + username + ", please try again.");
>          forward("/jsp/clients/internal/index.jsp");
>        } else {
>          String phase = "general";
>          String which = "0";          
>          LookupGeneral(c, which, request);
>          
>          // Forward along with client (c) and the information in the
>Lookup...
>          request.setAttribute("client", c);
>          request.setAttribute("phase", phase);
>          request.setAttribute("action", action);
>          request.setAttribute("which", which);
>          
>          forward("/jsp/clients/internal/index.jsp");
>        }
>      } catch (SQLException e) {
>        setError("SQLError viewing internal client information");
>        setExtraErr(e.getMessage() + " RootCause: " + e.getCause());
>        while ((e = e.getNextException()) != null) {
>          log("Next exception: " + e.getMessage());
>          log("Next exception major issue: " + e.getErrorCode());
>          log("Next exception major issue: " + e.getCause());
>        }
>        err();
>      } catch (Exception e) {
>        setError("General Error viewing internal client information");
>        setExtraErr(e.getMessage() + " RootCause: " + e.getCause());
>        err();
>      } finally {
>        try {
>          log("Closing connections");
>          if (conn != null) {
>            conn.close();
>            conn = null;
>          }
>          log("Closed");
>        } catch (SQLException e) { ; }
>      }
>    }
>  }
>
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
>For additional commands, e-mail: commons-user-help@jakarta.apache.org
>
>  
>



---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org


Mime
View raw message