svn commit: r1658734 - in /tomcat/trunk/java/org/apache/coyote/http11/upgrade: LocalStrings.properties UpgradeProcessor.java

2015-02-10 Thread markt 
Author: markt
Date: Tue Feb 10 15:43:46 2015
New Revision: 1658734

URL: http://svn.apache.org/r1658734
Log:
Ensure that a dropped connection does not leave references to the 
UpgradeProcessor

Modified:
tomcat/trunk/java/org/apache/coyote/http11/upgrade/LocalStrings.properties
tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeProcessor.java

Modified: 
tomcat/trunk/java/org/apache/coyote/http11/upgrade/LocalStrings.properties
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/upgrade/LocalStrings.properties?rev=1658734r1=1658733r2=1658734view=diff
==
--- tomcat/trunk/java/org/apache/coyote/http11/upgrade/LocalStrings.properties 
(original)
+++ tomcat/trunk/java/org/apache/coyote/http11/upgrade/LocalStrings.properties 
Tue Feb 10 15:43:46 2015
@@ -14,6 +14,8 @@
 # limitations under the License.
 
 upgradeProcessor.isCloseFail=Failed to close input stream associated with 
upgraded connection
+upgradeProcessor.onDataAvailableFail=Failed to process data available event
+upgradeProcessor.onWritePossibleFail=Failed to process write possible event
 upgradeProcessor.osCloseFail=Failed to close output stream associated with 
upgraded connection
 
 upgrade.sis.isFinished.ise=It is illegal to call isFinished() when the 
ServletInputStream is not in non-blocking mode (i.e. setReadListener() must be 
called first)

Modified: 
tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeProcessor.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeProcessor.java?rev=1658734r1=1658733r2=1658734view=diff
==
--- tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeProcessor.java 
(original)
+++ tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeProcessor.java 
Tue Feb 10 15:43:46 2015
@@ -98,10 +98,24 @@ public class UpgradeProcessor implements
 @Override
 public final SocketState upgradeDispatch(SocketStatus status) throws 
IOException {
 if (status == SocketStatus.OPEN_READ) {
-upgradeServletInputStream.onDataAvailable();
-upgradeServletOutputStream.checkWriteDispatch();
+try {
+upgradeServletInputStream.onDataAvailable();
+upgradeServletOutputStream.checkWriteDispatch();
+} catch (IOException ioe) {
+// The error handling within the ServletInputStream should have
+// marked the stream for closure which will get picked up 
below,
+// triggering the clean-up of this processor.
+
log.debug(sm.getString(upgradeProcessor.onDataAvailableFail), ioe);
+}
 } else if (status == SocketStatus.OPEN_WRITE) {
-upgradeServletOutputStream.onWritePossible();
+try {
+upgradeServletOutputStream.onWritePossible();
+} catch (IOException ioe) {
+// The error handling within the ServletOutputStream should 
have
+// marked the stream for closure which will get picked up 
below,
+// triggering the clean-up of this processor.
+
log.debug(sm.getString(upgradeProcessor.onWritePossibleFail), ioe);
+}
 } else if (status == SocketStatus.STOP) {
 try {
 upgradeServletInputStream.close();



-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Re: svn commit: r1658734 - in /tomcat/trunk/java/org/apache/coyote/http11/upgrade: LocalStrings.properties UpgradeProcessor.java

2015-02-10 Thread Rémy Maucherat 
2015-02-10 16:43 GMT+01:00 ma...@apache.org:

 Author: markt
 Date: Tue Feb 10 15:43:46 2015
 New Revision: 1658734

 URL: http://svn.apache.org/r1658734
 Log:
 Ensure that a dropped connection does not leave references to the
 UpgradeProcessor

 Good find, but what happens if onDataAvailable or onWritePossible throw a
runtime exception (like a NPE), since this is user code. It could also leak
then ? Shouldn't the code catch everything, also call onError on the
listener and close ?

Rémy

Re: svn commit: r1658734 - in /tomcat/trunk/java/org/apache/coyote/http11/upgrade: LocalStrings.properties UpgradeProcessor.java

2015-02-10 Thread Mark Thomas 
On 10/02/2015 15:59, Rémy Maucherat wrote:
 2015-02-10 16:43 GMT+01:00 ma...@apache.org:
 
 Author: markt
 Date: Tue Feb 10 15:43:46 2015
 New Revision: 1658734

 URL: http://svn.apache.org/r1658734
 Log:
 Ensure that a dropped connection does not leave references to the
 UpgradeProcessor

 Good find, but what happens if onDataAvailable or onWritePossible throw a
 runtime exception (like a NPE), since this is user code. It could also leak
 then ? Shouldn't the code catch everything, also call onError on the
 listener and close ?

Good point. I was thinking purely in terms of WebSocket but you are
right - this needs to be more general. I'll take another look.

Mark


-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org