Author: markt Date: Tue Feb 10 20:05:12 2015 New Revision: 1658790 URL: http://svn.apache.org/r1658790 Log: Make code calling user provided listeners more robust.
Modified: tomcat/trunk/java/org/apache/coyote/http11/upgrade/LocalStrings.properties tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeProcessor.java tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeServletInputStream.java tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeServletOutputStream.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=1658790&r1=1658789&r2=1658790&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 20:05:12 2015 @@ -14,16 +14,18 @@ # 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.errorCloseFail=Failed to close InputStream cleanly after a previous error 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) upgrade.sis.isReady.ise=It is illegal to call isReady() when the ServletInputStream is not in non-blocking mode (i.e. setReadListener() must be called first) +upgrade.sis.onErrorFail=onError processing for the registered ReadListener triggered this further error which was swallowed upgrade.sis.readListener.null=It is illegal to pass null to setReadListener() upgrade.sis.readListener.set=It is illegal to call setReadListener() more than once for the same upgraded connection upgrade.sis.read.ise=It is illegal to call any of the read() methods in non-blocking mode without first checking that there is data available by calling isReady() +upgrade.sos.errorCloseFail=Failed to close OutputStream cleanly after a previous error upgrade.sos.canWrite.ise=It is illegal to call canWrite() when the ServletOutputStream is not in non-blocking mode (i.e. setWriteListener() must be called first) +upgrade.sos.onErrorFail=onError processing for the registered WriteListener triggered this further error which was swallowed upgrade.sos.writeListener.null=It is illegal to pass null to setWriteListener() upgrade.sos.writeListener.set=It is illegal to call setWriteListener() more than once for the same upgraded connection upgrade.sis.write.ise=It is illegal to call any of the write() methods in non-blocking mode without first checking that there is space available by calling isReady() 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=1658790&r1=1658789&r2=1658790&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 20:05:12 2015 @@ -98,24 +98,10 @@ public class UpgradeProcessor implements @Override public final SocketState upgradeDispatch(SocketStatus status) { if (status == SocketStatus.OPEN_READ) { - 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); - } + upgradeServletInputStream.onDataAvailable(); + upgradeServletOutputStream.checkWriteDispatch(); } else if (status == SocketStatus.OPEN_WRITE) { - 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); - } + upgradeServletOutputStream.onWritePossible(); } else if (status == SocketStatus.STOP) { try { upgradeServletInputStream.close(); Modified: tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeServletInputStream.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeServletInputStream.java?rev=1658790&r1=1658789&r2=1658790&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeServletInputStream.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeServletInputStream.java Tue Feb 10 20:05:12 2015 @@ -21,12 +21,16 @@ import java.io.IOException; import javax.servlet.ReadListener; import javax.servlet.ServletInputStream; +import org.apache.juli.logging.Log; +import org.apache.juli.logging.LogFactory; +import org.apache.tomcat.util.ExceptionUtils; import org.apache.tomcat.util.net.SocketWrapperBase; import org.apache.tomcat.util.res.StringManager; public class UpgradeServletInputStream extends ServletInputStream { - protected static final StringManager sm = + private static final Log log = LogFactory.getLog(UpgradeServletInputStream.class); + private static final StringManager sm = StringManager.getManager(UpgradeServletInputStream.class); private final SocketWrapperBase<?> socketWrapper; @@ -176,7 +180,7 @@ public class UpgradeServletInputStream e } - protected final void onDataAvailable() throws IOException { + final void onDataAvailable() { if (listener == null) { return; } @@ -186,13 +190,16 @@ public class UpgradeServletInputStream e try { thread.setContextClassLoader(applicationLoader); listener.onDataAvailable(); + } catch (Throwable t) { + ExceptionUtils.handleThrowable(t); + onError(t); } finally { thread.setContextClassLoader(originalClassLoader); } } - protected final void onError(Throwable t) { + private final void onError(Throwable t) { if (listener == null) { return; } @@ -201,14 +208,24 @@ public class UpgradeServletInputStream e try { thread.setContextClassLoader(applicationLoader); listener.onError(t); + } catch (Throwable t2) { + ExceptionUtils.handleThrowable(t2); + log.warn(sm.getString("upgrade.sis.onErrorFail"), t2); } finally { thread.setContextClassLoader(originalClassLoader); } + try { + close(); + } catch (IOException ioe) { + if (log.isDebugEnabled()) { + log.debug(sm.getString("upgrade.sis.errorCloseFail"), ioe); + } + } ready = Boolean.FALSE; } - protected final boolean isCloseRequired() { + final boolean isCloseRequired() { return closeRequired; } } Modified: tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeServletOutputStream.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeServletOutputStream.java?rev=1658790&r1=1658789&r2=1658790&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeServletOutputStream.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeServletOutputStream.java Tue Feb 10 20:05:12 2015 @@ -21,6 +21,8 @@ import java.io.IOException; import javax.servlet.ServletOutputStream; import javax.servlet.WriteListener; +import org.apache.juli.logging.Log; +import org.apache.juli.logging.LogFactory; import org.apache.tomcat.util.ExceptionUtils; import org.apache.tomcat.util.net.DispatchType; import org.apache.tomcat.util.net.SocketWrapperBase; @@ -28,10 +30,11 @@ import org.apache.tomcat.util.res.String public class UpgradeServletOutputStream extends ServletOutputStream { - protected static final StringManager sm = + private static final Log log = LogFactory.getLog(UpgradeServletOutputStream.class); + private static final StringManager sm = StringManager.getManager(UpgradeServletOutputStream.class); - protected final SocketWrapperBase<?> socketWrapper; + private final SocketWrapperBase<?> socketWrapper; // Used to ensure that isReady() and onWritePossible() have a consistent // view of buffer and registered. @@ -114,7 +117,7 @@ public class UpgradeServletOutputStream } - protected final boolean isCloseRequired() { + final boolean isCloseRequired() { return closeRequired; } @@ -193,17 +196,22 @@ public class UpgradeServletOutputStream } - protected final void onWritePossible() throws IOException { - if (flushing) { - flushInternal(false, true); + final void onWritePossible() { + try { if (flushing) { - return; + flushInternal(false, true); + if (flushing) { + return; + } + } else { + // This may fill the write buffer in which case the + // isReadyForWrite() call below will re-register the socket for + // write + flushInternal(false, false); } - } else { - // This may fill the write buffer in which case the - // isReadyForWrite() call below will re-register the socket for - // write - flushInternal(false, false); + } catch (IOException ioe) { + onError(ioe); + return; } // Make sure isReady() and onWritePossible() have a consistent view @@ -225,6 +233,9 @@ public class UpgradeServletOutputStream try { thread.setContextClassLoader(applicationLoader); listener.onWritePossible(); + } catch (Throwable t) { + ExceptionUtils.handleThrowable(t); + onError(t); } finally { thread.setContextClassLoader(originalClassLoader); } @@ -232,7 +243,7 @@ public class UpgradeServletOutputStream } - protected final void onError(Throwable t) { + private final void onError(Throwable t) { if (listener == null) { return; } @@ -241,9 +252,19 @@ public class UpgradeServletOutputStream try { thread.setContextClassLoader(applicationLoader); listener.onError(t); + } catch (Throwable t2) { + ExceptionUtils.handleThrowable(t2); + log.warn(sm.getString("upgrade.sos.onErrorFail"), t2); } finally { thread.setContextClassLoader(originalClassLoader); } + try { + close(); + } catch (IOException ioe) { + if (log.isDebugEnabled()) { + log.debug(sm.getString("upgrade.sos.errorCloseFail"), ioe); + } + } } --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org