You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2015/02/10 16:43:47 UTC

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

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=1658734&r1=1658733&r2=1658734&view=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=1658734&r1=1658733&r2=1658734&view=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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


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

Posted by Mark Thomas <ma...@apache.org>.
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


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

Posted by Rémy Maucherat <re...@apache.org>.
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