Bug 1100486 - Web Socket close does not close underlying connection
Summary: Web Socket close does not close underlying connection
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: JBoss Enterprise Application Platform 6
Classification: JBoss
Component: Web
Version: 6.3.0
Hardware: Unspecified
OS: Unspecified
unspecified
urgent
Target Milestone: ER6
: EAP 6.3.0
Assignee: Rémy Maucherat
QA Contact: Radim Hatlapatka
Russell Dickenson
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-05-22 21:58 UTC by Stuart Douglas
Modified: 2014-06-28 15:27 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-06-28 15:27:54 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Bugzilla 1103596 0 unspecified CLOSED WebSocket using NIO2 doesn't properly close connections after certain errors 2021-02-22 00:41:40 UTC

Internal Links: 1103596

Description Stuart Douglas 2014-05-22 21:58:30 UTC
When a web socket close message has been sent JBoss Web should tear down the underlying TCP connection, however this does not actually happen. Even though the ServletOutputStream is closed, this close call does not close the underlying socket.

Comment 1 Rémy Maucherat 2014-05-23 08:51:39 UTC
There's a discrepancy with Tomcat's handling, so I'll try to resolve it.

Comment 2 Rémy Maucherat 2014-05-23 11:35:10 UTC
Tried to align the behavior in r2415.

Comment 3 Stuart Douglas 2014-05-23 12:27:20 UTC
The current code still does not work. I think that when an update is present the keepAlive field in org.apache.coyote.http11.Http11AbstractProcessor must be set to false. 

I did up a quick patch that set this to false if the upgrade header is present, and it appears to work. Not 100% sure if this is the best solution though.

Index: src/main/java/org/apache/coyote/http11/Http11NioProcessor.java
===================================================================
--- src/main/java/org/apache/coyote/http11/Http11NioProcessor.java	(revision 2415)
+++ src/main/java/org/apache/coyote/http11/Http11NioProcessor.java	(working copy)
@@ -1228,6 +1228,10 @@
 			keepAlive = false;
 		}

+        if(headers.getHeader("Upgrade") != null) {
+            keepAlive = false;
+        }
+
 		// If we know that the request is bad this early, add the
 		// Connection: close header.
 		keepAlive = keepAlive && !statusDropsConnection(statusCode);

Comment 4 Rémy Maucherat 2014-05-23 12:49:10 UTC
This doesn't look very good to me. I want to do a clean close, if it doesn't work, well ...

Comment 5 Stuart Douglas 2014-05-23 12:56:51 UTC
From what I can see the code you added does not do a clean close, it simply starts the next request processing, which does not really make sense, you should not be trying to parse a HTTP request on an upgraded connection. 

As far as I can tell setting keepAlive to false tells the server to not start processing the next request, but cleanly close the connection instead, which is exactly the behaviour that is expected.

Comment 6 Rémy Maucherat 2014-05-23 13:01:55 UTC
Yes, there needs to be some code to avoid doing a keepalive after upgrade. This is all Servlet 3.1 but not really tested obviously.

Comment 7 Rémy Maucherat 2014-05-23 14:15:19 UTC
The processing of the close now seems more appropriate, but need to make sure this does not go into keepalive (as with the diff above basically): r2416

Comment 8 Stuart Douglas 2014-05-23 14:31:53 UTC
The last commit fixes the issue.

Comment 9 Rémy Maucherat 2014-05-23 16:38:03 UTC
Ok, so it will be in the next web build.

Comment 10 Rémy Maucherat 2014-05-27 11:56:58 UTC
Included with https://bugzilla.redhat.com/show_bug.cgi?id=1100799

Comment 11 Radim Hatlapatka 2014-06-18 08:22:19 UTC
Verified in EAP 6.3.0.ER7


Note You need to log in before you can comment on or make changes to this bug.