Summary: | Stack overflow in connector | ||
---|---|---|---|
Product: | Tomcat 7 | Reporter: | Filip Hanik <fhanik> |
Component: | Connectors | Assignee: | Tomcat Developers Mailing List <dev> |
Status: | RESOLVED FIXED | ||
Severity: | normal | ||
Priority: | P2 | ||
Version: | 7.0.27 | ||
Target Milestone: | --- | ||
Hardware: | PC | ||
OS: | All |
Description
Filip Hanik
2012-06-13 03:51:40 UTC
The following are remarkable points in this stacktrace: > org.apache.tomcat.util.net.NioEndpoint.processSocket(NioEndpoint.java:730) > org.apache.tomcat.util.net.NioEndpoint$Poller.add(NioEndpoint.java:1008) NioEndpoint$Poller.add(NioChannel socket, int interestOps): [[[ if (close) { processSocket(socket, SocketStatus.STOP, false); } ]]] > org.apache.coyote.http11.Http11NioProtocol$Http11ConnectionHandler.longPoll(Http11NioProtocol.java:277) [[[ } else { //... socket.getSocket().getPoller().add(socket.getSocket()); ]]] so this happens with an application that uses Comet (those event() calls), when longPoll() is processed, but endpoint is being closed at the same time. It looks like SocketStatus.STOP value is not being honored. The poller#close flag is inaccessible from outside. Thanks for the analysis. Here is what I think will fix it Index: java/org/apache/coyote/http11/Http11NioProcessor.java =================================================================== --- java/org/apache/coyote/http11/Http11NioProcessor.java (revision 1349898) +++ java/org/apache/coyote/http11/Http11NioProcessor.java (working copy) @@ -155,7 +155,7 @@ rp.setStage(org.apache.coyote.Constants.STAGE_ENDED); - if (error) { + if (error || status==SocketStatus.STOP) { return SocketState.CLOSED; } else if (!comet) { if (keepAlive) { What this does, is that it prevents reuse of connections after a Comet transaction has been notified of a STOP event. I was thinking along the same lines but fixing it in the Adaptor rather than just for NIO. The issue with my fix is that it somewhat abused the error flag (by changing it's meaning to mean "close the connection for any reason" rather than "something went wrong". Your fix is clearer as to intention but it needs to be made to APR too. I'll so that now unless you beat me to it. Fixed for trunk and 7.0.x and will be included in 7.0.28 onwards. |