Ok ... so I took Ben's advice and decided to (at least for testing) yank
a bunch of code, and see about a slightly more modular approach. Also,
thanks to Antony, I am forwarding AFTER the database close. Here's what
I've found ....
Closing's were extracted into a method, and the while (conn == null) was
pulled out.
I also pulled everything but the following code [below]. In it, I have
yanked the DBCP code and am establishing a single connection in the
init(), which stays open until the destroy() method is called. Not a
great solution, but wanted to see if it was DBCP related.
As John pointed out, it is DBCP related (possibly) ... however, I can
generate the same error's (albeit, fewer of them) when using straight
JDBC Connections. For instance:
Scenario 1 (Using DBCP):
- Call the servlet (1 time)
- Redirected to JSP file(s)
- Display of information correctly [GOOD]
- Call the servlet (5 times)
- Redirected to JSP file(s)
- Display of information correctly 1 of the 5 times [BAD]
Scenario 2 (NOT Using DBCP):
- Call the servlet (1 time)
- Redirected to JSP file(s)
- Display of information correctly [GOOD]
- Call the servlet (5 times)
- Redirected to JSP file(s)
- Display of information correctly 3 of the 5 times [BAD,BETTER]
Scenario 3 (NOT Using DBCP, Simplified JSP file):
- Call the servlet (1 time)
- Redirected to JSP file(s)
- Display of information correctly [GOOD]
- Call the servlet (5 times)
- Redirected to JSP file(s)
- Display of information correctly 4 of the 5 times [BAD,BETTER]
I'm not using a JK connector, or Apache in my testing, just plain Tomcat
on port 8080.
However, what I'm beginning to wonder and note is:
a. Using DBCP IS causing some of the problem.
b. By using more simple JSP files, I can decrease the chance of
errors, but 4/5 is still not good. Is there a limit to the number of
active sessions, or amount of data I can put in the session, or
processing time (less than 1 or 2 seconds) that I'm missing? A workload
of 5 servlets && 5 JSP's (each calling about 3 other JSP's, except in
the simplified test) doesn't seem like a lot.
My modified code is below. Please NOTE ... I can tell by the logs that
it gets done with all JDBC calls, and does the forward, whereby it's
occasionally (but not consistently) dieing in the JSP files. In the
case of the simplified test, the JSP file pulls three session variables
and displays a true / false on the screen. I've checked with a grep
tool, and I'm not closing or accessing the connection in any way in the
JSPs.
This is frustrating ....... Thanks for everyone's help.
protected void processRequest(HttpServletRequest request,
HttpServletResponse response)
throws ServletException, java.io.IOException {
set(request, response);
setServlet("InternalClients");
// Create / validate the current session
session = request.getSession(true);
username = request.getRemoteUser();
try {
if (conn == null || conn.isClosed()) {
// Try again to fetch a new connection?
log("Connection was closed");
init();
}
// Did not make a valid connection for some reason, take to error
screen
if (conn == null) {
setError(DBNULL_ERR);
err();
doDBClose();
return;
} else {
// Should be a valid user
if (username == null) {
setError(NOUSER_ERR);
err();
doDBClose();
return;
}
}
log("Sure of connection ... should NEVER be closed.");
} catch (Exception e) {
setError(CONTEXT_ERR);
err();
doDBClose();
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();
doDBClose();
} 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 {
doDBClose();
}
} 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);
request.setAttribute("inventions", c.getInventions());
}
} 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 {
doDBClose();
}
forward("/jsp/clients/internal/test.jsp");
}
}
On Tue, 2003-11-25 at 02:24, Ben Walding wrote:
> 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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
