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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]






--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]



Reply via email to