Author: markt Date: Wed Oct 14 09:53:32 2015 New Revision: 1708572 URL: http://svn.apache.org/viewvc?rev=1708572&view=rev Log: I/O errors during application initiated read/writes need to be handled at the point they occur since the application may swallow them. I/O exceptions reported by the application may not related to the connection and should not be treated as fatal to it.
Modified: tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java tomcat/trunk/java/org/apache/coyote/http2/Stream.java tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java Modified: tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java?rev=1708572&r1=1708571&r2=1708572&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java (original) +++ tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java Wed Oct 14 09:53:32 2015 @@ -396,7 +396,6 @@ public class Http2UpgradeHandler extends socketWrapper.write(true, fixedPayload, 0, 8); socketWrapper.flush(true); } - } } } @@ -491,9 +490,13 @@ public class Http2UpgradeHandler extends log.debug(target.limit() + " bytes"); } ByteUtil.set31Bits(header, 5, stream.getIdentifier().intValue()); - socketWrapper.write(true, header, 0, header.length); - socketWrapper.write(true, target.array(), target.arrayOffset(), target.limit()); - socketWrapper.flush(true); + try { + socketWrapper.write(true, header, 0, header.length); + socketWrapper.write(true, target.array(), target.arrayOffset(), target.limit()); + socketWrapper.flush(true); + } catch (IOException ioe) { + handleAppInitiatedIOException(ioe); + } } } } @@ -524,15 +527,40 @@ public class Http2UpgradeHandler extends } } ByteUtil.set31Bits(header, 5, stream.getIdentifier().intValue()); - socketWrapper.write(true, header, 0, header.length); - socketWrapper.write(true, data.array(), data.arrayOffset() + data.position(), - len); - socketWrapper.flush(true); + try { + socketWrapper.write(true, header, 0, header.length); + socketWrapper.write(true, data.array(), data.arrayOffset() + data.position(), + len); + socketWrapper.flush(true); + } catch (IOException ioe) { + handleAppInitiatedIOException(ioe); + } } } - void writeWindowUpdate(Stream stream, int increment) throws IOException { + /* + * Handles an I/O error on the socket underlying the HTTP/2 connection when + * it is triggered by application code (usually reading the request or + * writing the response). Such I/O errors are fatal so the connection is + * closed. The exception is re-thrown to make the client code aware of the + * problem. + * + * Note: We can not rely on this exception reaching the socket processor + * since the application code may swallow it. + */ + private void handleAppInitiatedIOException(IOException ioe) throws IOException { + close(); + throw ioe; + } + + + /* + * Needs to know if this was application initiated since that affects the + * error handling. + */ + void writeWindowUpdate(Stream stream, int increment, boolean applicationInitiated) + throws IOException { synchronized (socketWrapper) { // Build window update frame for stream 0 byte[] frame = new byte[13]; @@ -542,8 +570,16 @@ public class Http2UpgradeHandler extends socketWrapper.write(true, frame, 0, frame.length); // Change stream Id and re-use ByteUtil.set31Bits(frame, 5, stream.getIdentifier().intValue()); - socketWrapper.write(true, frame, 0, frame.length); - socketWrapper.flush(true); + try { + socketWrapper.write(true, frame, 0, frame.length); + socketWrapper.flush(true); + } catch (IOException ioe) { + if (applicationInitiated) { + handleAppInitiatedIOException(ioe); + } else { + throw ioe; + } + } } } @@ -997,7 +1033,7 @@ public class Http2UpgradeHandler extends ConnectionException, IOException { Stream stream = getStream(streamId, true); // +1 is for the payload byte used to define the padding length - writeWindowUpdate(stream, paddingLength + 1); + writeWindowUpdate(stream, paddingLength + 1, false); } Modified: tomcat/trunk/java/org/apache/coyote/http2/Stream.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/Stream.java?rev=1708572&r1=1708571&r2=1708572&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http2/Stream.java (original) +++ tomcat/trunk/java/org/apache/coyote/http2/Stream.java Wed Oct 14 09:53:32 2015 @@ -568,7 +568,7 @@ public class Stream extends AbstractStre // Increment client-side flow control windows by the number of bytes // read - handler.writeWindowUpdate(Stream.this, written); + handler.writeWindowUpdate(Stream.this, written, true); return written; } Modified: tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java?rev=1708572&r1=1708571&r2=1708572&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java (original) +++ tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java Wed Oct 14 09:53:32 2015 @@ -426,8 +426,6 @@ public class StreamProcessor extends Abs public SocketState process(SocketWrapperBase<?> socket) throws IOException { try { adapter.service(request, response); - } catch (IOException ioe) { - setErrorState(ErrorState.CLOSE_CONNECTION_NOW, ioe); } catch (Exception e) { setErrorState(ErrorState.CLOSE_NOW, e); } --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org