You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2015/02/18 10:25:30 UTC
svn commit: r1660582 - in /tomcat/trunk/java/org/apache:
coyote/http11/upgrade/UpgradeServletInputStream.java
coyote/http11/upgrade/UpgradeServletOutputStream.java
tomcat/util/net/Nio2Endpoint.java
Author: remm
Date: Wed Feb 18 09:25:30 2015
New Revision: 1660582
URL: http://svn.apache.org/r1660582
Log:
- Add debugging of my own (sorry), on socket close.
- Try dropping the direct socket close from the upgraded streams (the sockets seem to still be closed and the ws tests pass with NIO1+2). Actually following the discussions on the EG, I don't think the close required logic is fully right, it would seem it should wait for both input AND output to be closed, but this needs to be validated.
Modified:
tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeServletInputStream.java
tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeServletOutputStream.java
tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java
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=1660582&r1=1660581&r2=1660582&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeServletInputStream.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeServletInputStream.java Wed Feb 18 09:25:30 2015
@@ -142,7 +142,6 @@ public class UpgradeServletInputStream e
@Override
public void close() throws IOException {
closeRequired = true;
- socketWrapper.close();
}
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=1660582&r1=1660581&r2=1660582&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeServletOutputStream.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/upgrade/UpgradeServletOutputStream.java Wed Feb 18 09:25:30 2015
@@ -172,7 +172,6 @@ public class UpgradeServletOutputStream
@Override
public void close() throws IOException {
closeRequired = true;
- socketWrapper.close();
}
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java?rev=1660582&r1=1660581&r2=1660582&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java Wed Feb 18 09:25:30 2015
@@ -589,6 +589,10 @@ public class Nio2Endpoint extends Abstra
}
public void closeSocket(SocketWrapperBase<Nio2Channel> socket) {
+ if (log.isDebugEnabled()) {
+ log.debug("Calling [" + this + "].closeSocket([" + socket + "],[" + socket.getSocket() + "])",
+ new Exception());
+ }
if (socket == null) {
return;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1660582 - in /tomcat/trunk/java/org/apache:
coyote/http11/upgrade/UpgradeServletInputStream.java coyote/http11/upgrade/UpgradeServletOutputStream.java
tomcat/util/net/Nio2Endpoint.java
Posted by Rémy Maucherat <re...@apache.org>.
2015-02-18 10:45 GMT+01:00 Mark Thomas <ma...@apache.org>:
> On 18/02/2015 09:25, remm@apache.org wrote:
> > Author: remm
> > Date: Wed Feb 18 09:25:30 2015
> > New Revision: 1660582
> >
> > URL: http://svn.apache.org/r1660582
> > Log:
> > - Add debugging of my own (sorry), on socket close.
> > - Try dropping the direct socket close from the upgraded streams (the
> sockets seem to still be closed and the ws tests pass with NIO1+2).
>
> That should be fine. The socket will get closed in
> UpgradeProcessor.upgradeDispatch()
>
I noticed in the test runs that the close debug was still there, so it
looked relatively fine.
>
> > Actually following the discussions on the EG, I don't think the close
> required logic is fully right, it would seem it should wait for both input
> AND output to be closed, but this needs to be validated.
>
> I agree the close logic is wrong. I wanted to fix the existing issues
> before I started on this. Looking at the code, it shouldn't be too hard
> to fix.
>
> The stream close could cause calling shutdownInput and shutdownOutput.
Switching to:
if (upgradeServletInputStream.isCloseRequired() &&
upgradeServletOutputStream.isCloseRequired()) {
return SocketState.CLOSED;
}
in UpgradeProcessor doesn't work (the socket close occurs during shutdown
during the tests).
Rémy
Re: svn commit: r1660582 - in /tomcat/trunk/java/org/apache: coyote/http11/upgrade/UpgradeServletInputStream.java
coyote/http11/upgrade/UpgradeServletOutputStream.java tomcat/util/net/Nio2Endpoint.java
Posted by Mark Thomas <ma...@apache.org>.
On 18/02/2015 09:25, remm@apache.org wrote:
> Author: remm
> Date: Wed Feb 18 09:25:30 2015
> New Revision: 1660582
>
> URL: http://svn.apache.org/r1660582
> Log:
> - Add debugging of my own (sorry), on socket close.
> - Try dropping the direct socket close from the upgraded streams (the sockets seem to still be closed and the ws tests pass with NIO1+2).
That should be fine. The socket will get closed in
UpgradeProcessor.upgradeDispatch()
> Actually following the discussions on the EG, I don't think the close required logic is fully right, it would seem it should wait for both input AND output to be closed, but this needs to be validated.
I agree the close logic is wrong. I wanted to fix the existing issues
before I started on this. Looking at the code, it shouldn't be too hard
to fix.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org