svn commit: r1661642 - /tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeProcessor.java
Author: markt
Date: Mon Feb 23 11:26:56 2015
New Revision: 1661642
URL: http://svn.apache.org/r1661642
Log:
Don't default to an infinite timeout for write. Generally, writes should happen
(fairly) quickly.
Modified:
tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeProcessor.java
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=1661642r1=1661641r2=1661642view=diff
==
--- tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeProcessor.java
(original)
+++ tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeProcessor.java
Mon Feb 23 11:26:56 2015
@@ -54,8 +54,13 @@ public class UpgradeProcessor implements
this.upgradeServletOutputStream = new
UpgradeServletOutputStream(wrapper);
wrapper.unRead(leftOverInput);
+/*
+ * Infinite read timeouts make sense since it is unknown how quickly -
+ * or even if at all - a client will send data. However, leave the
write
+ * timeout alone since when the server writes to the client that should
+ * always happen within the configured timeout.
+ */
wrapper.setReadTimeout(INFINITE_TIMEOUT);
-wrapper.setWriteTimeout(INFINITE_TIMEOUT);
if (httpUpgradeHandler instanceof InternalHttpUpgradeHandler) {
wrapper.setInternalUpgrade(true);
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: svn commit: r1661642 - /tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeProcessor.java
2015-02-23 13:32 GMT+01:00 Mark Thomas ma...@apache.org: I'm not sure if that is fixing the symptom or the cause. I'm still seeing something going wrong with what should be a clean shutdown in TestWsWebSocketContainer.testMaxMessageSize04(). I took a look at the source for BufferedOutputStream and that does flush on close. I think ServletOutputStream should behave the same way. Disabling the flush might give us some insight into what else might be going wrong but I don't think disabling is a long term solution. I'm not saying to disable fushing, but to do a non blocking flush (see r1661652). If there's a backlog (or a client not reading like for TestWebSocketFrameClientSSL testBug56032 this did end up waiting forever somehow (which didn't happen before). With the infinite write timeout removed, I have the following tests failing due to a race between the real IO and the Websockets timeout: TestWsWebSocketContainer Testcase: testWriteTimeoutServerEndpoint took 9.836 sec FAILED expected:class java.net.SocketTimeoutException but was:class java.io.EOFException Testcase: testWriteTimeoutServerContainer took 9.808 sec FAILED expected:class java.net.SocketTimeoutException but was:class java.io.EOFException IMO, we're fine with the infinite write timeout (for now), or it would need to be more than what it is right now. Rémy
Re: svn commit: r1661642 - /tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeProcessor.java
On 23/02/2015 13:24, Rémy Maucherat wrote: 2015-02-23 13:32 GMT+01:00 Mark Thomas ma...@apache.org: I'm not sure if that is fixing the symptom or the cause. I'm still seeing something going wrong with what should be a clean shutdown in TestWsWebSocketContainer.testMaxMessageSize04(). I took a look at the source for BufferedOutputStream and that does flush on close. I think ServletOutputStream should behave the same way. Disabling the flush might give us some insight into what else might be going wrong but I don't think disabling is a long term solution. I'm not saying to disable fushing, but to do a non blocking flush (see r1661652). If there's a backlog (or a client not reading like for TestWebSocketFrameClientSSL testBug56032 this did end up waiting forever somehow (which didn't happen before). With the infinite write timeout removed, I have the following tests failing due to a race between the real IO and the Websockets timeout: TestWsWebSocketContainer Testcase: testWriteTimeoutServerEndpoint took 9.836 sec FAILED expected:class java.net.SocketTimeoutException but was:class java.io.EOFException Testcase: testWriteTimeoutServerContainer took 9.808 sec FAILED expected:class java.net.SocketTimeoutException but was:class java.io.EOFException IMO, we're fine with the infinite write timeout (for now), or it would need to be more than what it is right now. There is a combination of things going on here. With infinite timeout + blocking flush there is the possibility of an infinite wait. Clearly that is wrong. At socket close, I do think the flush needs to be blocking (with a sensible timeout) rather than non-blocking. The app should be in control of how long to wait to close the socket before closing it anyway and losing data. I'm currently thinking of setting the write timeout to the connection timeout followed by blocking flush on close. I'll revert the change to the write timeout. I'm also close to what is causing the problem with testMaxMessageSize04(). There is a race between WsSession.onClose() blocking write being completed. I want to take a little more time to make sure I fix that properly. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: svn commit: r1661642 - /tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeProcessor.java
On 23/02/2015 12:07, Rémy Maucherat wrote: 2015-02-23 12:26 GMT+01:00 ma...@apache.org: Author: markt Date: Mon Feb 23 11:26:56 2015 New Revision: 1661642 URL: http://svn.apache.org/r1661642 Log: Don't default to an infinite timeout for write. Generally, writes should happen (fairly) quickly. This is messing up some tests for me. Although I don't understand why it was working before, I indeed have a NIO2 test hanging on close (the TestWebSocketFrameClientSSL one with the error and the backlog). The close actually does a blocking write, which I tried removing successfully. Can we try that instead ? I'm not sure if that is fixing the symptom or the cause. I'm still seeing something going wrong with what should be a clean shutdown in TestWsWebSocketContainer.testMaxMessageSize04(). I took a look at the source for BufferedOutputStream and that does flush on close. I think ServletOutputStream should behave the same way. Disabling the flush might give us some insight into what else might be going wrong but I don't think disabling is a long term solution. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: svn commit: r1661642 - /tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeProcessor.java
2015-02-23 12:26 GMT+01:00 ma...@apache.org: Author: markt Date: Mon Feb 23 11:26:56 2015 New Revision: 1661642 URL: http://svn.apache.org/r1661642 Log: Don't default to an infinite timeout for write. Generally, writes should happen (fairly) quickly. This is messing up some tests for me. Although I don't understand why it was working before, I indeed have a NIO2 test hanging on close (the TestWebSocketFrameClientSSL one with the error and the backlog). The close actually does a blocking write, which I tried removing successfully. Can we try that instead ? Rémy
