Bug 51181 - Add support for Web Sockets
Summary: Add support for Web Sockets
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 7
Classification: Unclassified
Component: Connectors (show other bugs)
Version: trunk
Hardware: PC All
: P2 enhancement with 65 votes (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-10 13:28 UTC by Graeme Rocher
Modified: 2014-09-23 10:34 UTC (History)
11 users (show)



Attachments
Origin and protocol client handshake TODOs (21.26 KB, patch)
2012-02-04 01:48 UTC, Johno Crawford
Details | Diff
Fragmentation control frames (54.91 KB, patch)
2012-02-16 17:47 UTC, Johno Crawford
Details | Diff
Autobahn fail threaddump (15.54 KB, text/plain)
2012-02-28 11:43 UTC, Johno Crawford
Details
Introduce status code constants (13.13 KB, patch)
2012-02-29 15:12 UTC, Johno Crawford
Details | Diff
Overrides for connection open and close (4.87 KB, patch)
2012-03-01 19:13 UTC, Johno Crawford
Details | Diff
Improved echo example (6.23 KB, patch)
2012-03-02 12:53 UTC, Johno Crawford
Details | Diff
Overrides for connection open and close (6.08 KB, patch)
2012-03-02 15:39 UTC, Johno Crawford
Details | Diff
Connection close not called on IOE (5.36 KB, patch)
2012-03-04 12:12 UTC, Johno Crawford
Details | Diff
Multiplayer snake example (27.24 KB, patch)
2012-03-05 00:48 UTC, Johno Crawford
Details | Diff
Multiplayer snake example (27.84 KB, patch)
2012-03-05 09:26 UTC, Johno Crawford
Details | Diff
Multiplayer snake collision fix (10.48 KB, patch)
2012-03-06 21:54 UTC, Johno Crawford
Details | Diff
Chat example collision fix and cleanup (16.19 KB, patch)
2012-03-12 21:59 UTC, Johno Crawford
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Graeme Rocher 2011-05-10 13:28:23 UTC
Both Jetty (http://blogs.webtide.com/gregw/entry/jetty_websocket_server) and Glass fish (http://antwerkz.com/glassfish-web-sockets-sample/) have native support for Web Sockets.

It would be great if Tomcat provided native Web Socket compatibilities to avoid the need to run two servers
Comment 1 Mark Thomas 2011-05-10 13:32:03 UTC
A significant factor in enhancement prioritisation (for me at least) is user demand. I don't recall seeing much demand for this from the Tomcat user community.

Voting for an issue is the clearest way to register your interest in a particular enhancement.
Comment 2 Graeme Rocher 2011-05-10 13:41:29 UTC
There is significant interest in this area from the Grails community, which uses Tomcat as the default container. Also some general citations from around the web:

* Comments on this blog post from Tomcat users asking for Web Sockets

http://www.tomcatexpert.com/blog/2010/06/29/apache-tomcat-7-has-been-released

* Comments from users on InfoQ article announcing Tomcat 7 about Web Sockets

http://www.infoq.com/news/2010/07/tomcat_7

* Question from user on Tomcat mailing list

http://old.nabble.com/Tomcat-vs.-web-sockets-td30454648.html

Current examples of Java applications using Web Sockets on Cloud Foundry also rely on Node.js for Web Socket support

Obviously it is new technology, but it is growing fast and I would recommend planning a strategy for it soon (Jetty added Web Sockets in 2009, and GlassFish in 2010)
Comment 3 Nikolas 2011-05-10 19:38:12 UTC
You also have to consider the fact that a lot of people are starting *new* projects that use websockets and the first thing you do when starting a new project is search for a server that will support it. I think people pass up using tomcat simply because there is no sign of it being implemented here whereas glassfish is a quick and easy alternative. The point is, even if it would be easier to use tomcat as a solution in some cases, its not even worth asking for it when there are viable alternatives. 

Websockets are new and therefore the projects that use websockets are new. I'd be willing to bet fewer people are going to "port" their application to websockets than those who are going to start a new one that use websockets. If i'm right, Tomcat will be an afterthought until it gets put in (and it would explain the "lack of interest").

If you don't support websockets, tomcat will start losing serious ground.
Comment 4 Julian Reschke 2011-05-11 08:01:29 UTC
For the record, it's not "HTML5" Web Sockets.

The protocol is being defined by the IETF HyBi Working Group, and is currently approaching Last Call.

See <http://tools.ietf.org/html/draft-ietf-hybi-thewebsocketprotocol-07>
Comment 5 Graeme Rocher 2011-05-11 08:28:53 UTC
Gotcha. The two topics (HTML5 and Web Sockets) are normally discussed together in articles, blogs etc. hence my confusion.
Comment 6 Mark Thomas 2011-05-19 11:04:46 UTC
Current thinking on this is that Tomcat would provide the SocketIO API in a similar manner to how Comet is supported. The underlying transports, e.g web sockets, will also be exposed if practical.
Comment 7 Filip Hanik 2011-05-20 14:39:57 UTC
true, web sockets are really nothing more than comet, with some additional packet info, and that could be deal with using an InputFilter
Comment 8 Nikolas 2011-05-20 15:34:44 UTC
@Mark Thomas
"If practical". Heh

@Filip Hanik
It is not the same thing as Comet, clearly you've never actually done any development with Comet or Websockets. I can't have you polluting this thread with misinformation. The overhead in both server resources, latency, and throughput makes Comet laughable in comparison to websockets. Its an "ok" fallback solution for asynchronous communication but it's by no means on the same level as websockets.
Comment 9 Filip Hanik 2011-05-20 22:20:21 UTC
(In reply to comment #8)
> @Filip Hanik
> It is not the same thing as Comet, clearly you've never actually done any
> development with Comet or Websockets. I can't have you polluting this thread
> with misinformation. The overhead in both server resources, latency, and
> throughput makes Comet laughable in comparison to websockets. Its an "ok"
> fallback solution for asynchronous communication but it's by no means on the
> same level as websockets.

misinformed, I like that. 

I was commenting from an implementation standpoint at the connector level.
We can piggy back on the low level comet notifications we have implemented in the endpoints.

At the processor level, we would need an inputfilter to handle package dissemination.

and at the servlet level, provide new interfaces for DataPackets

before you call people misinformed, read the source code of tomcat, read the websocket spec, and you'll arrive at the same conclusion.

now, go and inform yourself!!
Comment 10 Pid 2011-05-22 20:29:54 UTC
> I can't have you polluting this thread with misinformation.

Maybe you need a superinjunction. ;)
Comment 11 Antoine Roux 2011-05-26 20:07:48 UTC
Hi,
This is definitely something I would love to see in Tomcat! Websockets will allow to build richer web application and all major browsers already support them.
Comment 12 gootdude 2011-06-30 02:23:31 UTC
Please add Web Sockets to Tomcat.

The functionality provided is incredible and finally allows our applications to provide real time communication.

Instead of resorting to running multiple servlet containers, we're switching off tomcat completely since there does not seem to be any sign of sockets coming any time soon.

I understand user demand drives the functionality, but I think we can agree that web sockets are a dream for web applications.
Comment 13 Mark Thomas 2011-09-06 23:15:59 UTC
It might appear that there hasn't been much progress on this but there has been a lot going on behind the scenes.

1. The HTTP connectors have been refactored to reduce duplication and inconsistenceies between them that should make it possible to implement WebSockets once for all three HTTP connectors rather than once for each connector. This was long overdue and it took rather a long time to unpick the various differences that had crept in over the years.

2. I took a look at a SocketIO implementation and I have some initial code but the API doesn't translate that easily (at least I found it didn't) from JavaScript to Java. Therefore I'm not convinced at the moment that this is the right direction to go in.

What would be very useful at this point is an idea of the API users would like to work with. I can make something up but it would be much better if this was user driven. Proposals can be attached to this Bugzilla issue.
Comment 14 Harikrishnadhas Krishnaraj 2011-09-26 13:06:20 UTC
I'm Eagerly looking out tomcat to implement the Web Socket....
Comment 15 Mike Slinn 2011-09-27 06:11:45 UTC
+1 for WebSockets support!
Comment 16 Manuel Vergel 2011-10-18 10:00:11 UTC
+ 1 too. Greatly needed in my new projects.
Comment 17 jfclere 2011-11-09 21:13:06 UTC
See http://svn.apache.org/repos/asf/labs/monsoon/trunk/ too
Comment 18 Pid 2011-11-10 11:56:01 UTC
Monsoon seems to have stalled somewhat
Comment 19 Suresh Karuppannan 2011-11-16 12:06:59 UTC
+1, I'm looking into use it in our new product and I always prefer Tomcat.
Comment 20 Stein Morten Hugubakken 2011-12-07 10:11:37 UTC
+1
Comment 21 Robert Oschwald 2011-12-23 14:49:21 UTC
+1
Comment 22 goberman 2012-01-05 22:33:32 UTC
Mark,
Is there any estimate when WebSockets support will be implemented?

As far as API, I would not mind if it is somewhat similar to the Comet functionality. In fact it would be nice if the same Servlet instance could handle both WebSocket and Comet requests.

So it may looks something like:

public class WebFrameworkServlet extends HttpServlet implements WebSocketProcessor /*, CometProcessor*/ {


  /*
  // comet event processor
  public void event(CometEvent event) throws IOException, ServletException {
    HttpServletRequest request = event.getHttpServletRequest();
    HttpServletResponse response = event.getHttpServletResponse();
    if (event.getEventType() == CometEvent.EventType.BEGIN) {
      ...
    }
    ...
  }
  */

  // WebSocket event processor
  public void event(WebSocketEvent event) throws IOException, ServletException {
    WebSocketServletRequest request = event.getHttpServletRequest();
    WebSocketServletResponse response = event.getHttpServletResponse();
    // initial connection
    if (event.getEventType() == WebSocketEvent.EventType.BEGIN) {
      ...
    }
    // 
    if (event.getEventType() == WebSocketEvent.EventType.MESSAGE) {
      // Get message
      WebSocketMessage inboundMessage = request.getMessage();
      // Send message back to the client
      response.sendMessage(new WebSocketMessage(...));
      ...
    }
    ...
  }
  ...
}

One longstanding limitation to the Tomcat Comet API is the ability to specify "maximum send queue size threshold" to disconnect slow consumers when send backlog crosses a limit. I have requested this in the past with no luck. This is is absolutely critical for the production application. We had situations when a slow consumer would block IO thread dedicated to multiple clients eventually causing Tomcat run out of memory.
Comment 23 Mark Thomas 2012-01-08 13:03:42 UTC
(In reply to comment #22)
> Is there any estimate when WebSockets support will be implemented?

No.

The preparatory work of aligning the connectors is pretty much complete which will make implementation easier. There is some ongoing discussion in Servlet 3.1 EG on how Servlet 3.1 and Web Sockets will work together.

I have some rough ideas implemented that I really should get around to sharing. Nag me (on the dev list) in a couple of weeks if I haven't done anything.

> As far as API, I would not mind if it is somewhat similar to the Comet
> functionality. In fact it would be nice if the same Servlet instance could
> handle both WebSocket and Comet requests.

As long as it doesn't create additional complexities, that should be possible.

> So it may looks something like:

Thanks for the API suggestion. It is good to see someone finally responding my request from September. Further input from other potential users welcome. Jetty have offered to work on a joint API - there is clearly some benefit in that.

> One longstanding limitation to the Tomcat Comet API is the ability to specify
> "maximum send queue size threshold" to disconnect slow consumers when send
> backlog crosses a limit. I have requested this in the past with no luck. This
> is is absolutely critical for the production application. We had situations
> when a slow consumer would block IO thread dedicated to multiple clients
> eventually causing Tomcat run out of memory.

That is really a separate issue. The best way to approach that is to create an enhancement request in Bugziila. Enhancement requests with test cases and patches tend to get looked at faster.
Comment 24 goberman 2012-01-08 22:38:44 UTC
I actually played with Jetty WebSocket API last Friday and was able to embed it into Tomcat quite easily. The only issue is that it has to be on a separate port.
As I mentioned before, it would be a good selling point if Tomcat could handle WebSockets, Comet, and "normal" http requests from a single port.

As far as controlling client send queue, I have already requested it over a year ago: https://issues.apache.org/bugzilla/show_bug.cgi?id=50495. It was suggested that I propose a patch and the request was closed with "will not fix" status. I was given some pointers on how it works, but did not think that patching Tomcat source code is a good long term solution for us. I thought that now will be a good time to address it for both WebSockets and Tomcat Comet since the underlying plumbing is getting reworked anyway and may be shared. 
By the way, Grizzly framework actually does have the ability to limit client send queue with a single API method call.
Thanks
Comment 25 Pid 2012-01-10 09:35:09 UTC
>   // WebSocket event processor
>   public void event(WebSocketEvent event) throws IOException, ServletException
> {
>     WebSocketServletRequest request = event.getHttpServletRequest();
>     WebSocketServletResponse response = event.getHttpServletResponse();
>     // initial connection
>     if (event.getEventType() == WebSocketEvent.EventType.BEGIN) {
>       ...
>     }
>     // 
>     if (event.getEventType() == WebSocketEvent.EventType.MESSAGE) {
>       // Get message
>       WebSocketMessage inboundMessage = request.getMessage();
>       // Send message back to the client
>       response.sendMessage(new WebSocketMessage(...));

The server end probably also needs to be able to handle the receipt/issue of a WebSocketFrame (e.g. PING/PONG frames). It's possible to send a CLOSE event in between other frames and you might want to clean up before shutdown.

Sidenote: I think a Jetty & Tomcat collaboration would be pragmatic.

IMHO extending javax.servlet.http.WebSocketServlet would be a good final outcome.
Comment 26 Mark Thomas 2012-01-18 22:35:07 UTC
I dug out my old ideas and have been updating them. You can see how far I have got in [1]. Note that this is all very early days and pretty much everything is subject to change and a lot of the detail is missing. That said, I think the broad strokes are heading in the right direction.

Key points are:
- Limited connector (BIO/NIO/APR) specific code (thanks to the previous re-factoring)
- Write apps by extending WebSocketServlet
- Also interface to implement / base class to extend for reading / writing
- Provide stream-based and message-based APIs (stream will be implemented first and then message-based with configurable buffers on top off the streaming API)
- message-based API likely to be similar to Jetty's - will align to common API if at all possible
- reading/writing will use blocking IO per message (much, much simpler to implement and only the NIO connector could do non-blocking - could possibly add non-blocking IO later if there is sufficient demand)

As soon as I have something that roughly works end to end (i.e. I can get the WebSocket equivalent of HelloWorld working) I'll commit this to trunk with the aim of back-porting it to 7.0.x once the integration points are stable. It looks as if the integration points are going to be few and well defined so they should be stable pretty quickly which bodes well for getting this into 7.0.x.

The message-based API isn't dissimilar to the Comet-style API proposed above but rather than a single call-back with different event types, there will be one call-back per event type (close, text message, binary message). Control messages will be handled at a lower level.

One caveat and I can't stress this enough. The devil will be in the detail and experience with the Servlet 3.0 async work has shown that what appears to be a complete, working implementation may need a major re-write to make it stable. It isn't beyond the realm of possibility that there is a huge logical flaw in the approach I have taken so far that necessitates ripping most of this up and starting again.

Finally, there has been an awful lot of interest in this enhancement request but not much in the way of contributions. Hopefully as the code starts to get committed, contributions will be easier. Discussions on the patch are best held on the dev mailing list. I only wrote this progress report in BZ in case folks who are cc'd on the bug aren't on the dev list.

[1] http://people.apache.org/~markt/patches/draft/2012-01-18-websocket.patch
Comment 27 Toub 2012-01-20 22:36:59 UTC
Thanks for the report!

Will try to deal with embedded jetty until a first version is released.

Definitely a must-have feature in Tomcat.
Comment 28 Mark Thomas 2012-02-01 10:13:31 UTC
An initial implementation has been added to trunk. Once the major TODOs have been addressed, it will be back-ported to 7.0.x.
Comment 29 Johno Crawford 2012-02-04 01:48:20 UTC
Created attachment 28264 [details]
Origin and protocol client handshake TODOs

Thought it might be helpful to try implementing a few TODOs, please let me know whether or not you agree with the approach for origin and protocol.
Comment 30 Mark Thomas 2012-02-08 08:33:37 UTC
The origin changes look good. I'll incorporate them shortly.

Regarding the changes to createWebSocketInbound, I understand passing the protocol but why pass the HttpServletRequest as well?

Regarding the changes to the tests, I know we refer to them as unit tests but we frequently test more widely than that to provide greater confidence in the end to end processing chain. Unfortunately, that means we are going to need to write some (very basic) form of WebSocket client to test things.
Comment 31 Johno Crawford 2012-02-08 10:37:53 UTC
Flexibility more than anything, the developer may require access to the request in order to determine / set the initial state of the socket or application. I will try and look into writing a very basic WebSocket client this week if you do not beat me to it.
Comment 32 Adam Walczak 2012-02-09 23:53:16 UTC
PS. Oracle just put out JSR-356 for WebSockect:
http://jcp.org/en/jsr/detail?id=356
might come in handy for this issue
Comment 33 Pid 2012-02-10 09:45:41 UTC
(In reply to comment #31)
> Flexibility more than anything, the developer may require access to the request
> in order to determine / set the initial state of the socket or application.

4.2.1.10 says "Optionally, other header fields, such as those used to send cookies or request authentication to a server." Would just the header map be enough?

Given that these are potentially long running requests, keeping the original request object around might not be such a good idea. Would passing in a minimal or partial object copy be better?

You wouldn't want to be able to permit a write to the ServletOutputStream for example.
Comment 34 Johno Crawford 2012-02-10 11:20:39 UTC
(In reply to comment #33)
> Given that these are potentially long running requests, keeping the original
> request object around might not be such a good idea. Would passing in a minimal
> or partial object copy be better?
> 
> You wouldn't want to be able to permit a write to the ServletOutputStream for
> example.

Well put, I see where you are coming from. Unmodifiable map / copy of the headers would be fine from my perspective, of course if someone else can think of any other possible scenarios please comment :)
Comment 35 Lucas Dreyer 2012-02-13 13:53:52 UTC
+1, Tomcat is an integral part of our system and Web Socket support will be awesome.
Comment 36 Johno Crawford 2012-02-16 17:47:38 UTC
Created attachment 28342 [details]
Fragmentation control frames

Attaching patch sent by Petr Praus (mailing list).
Comment 37 Mark Thomas 2012-02-16 22:35:46 UTC
I can see the possible requirement for access to the request headers when verifying the origin (and a few other processes). However, I'm leaning towards not implementing that now and adding it later if there is a demand for it. If we go that route I do think we just need prevent access to the request since the problems that would start if a reference to the request is retained are not pretty. I'd probably just wrap the request in a facade that exposed the headers in read-only form and nothing else.

I'm now looking at the sub-protocol parts. The good news is that protocol names are token which makes parsing them simple (the approach used in the patch is fine - just not the way I would have done it). However, my reading of the specification is slightly different from the patch as implemented. There are two differences:
a) I see no requirement for the server to respect the clients preference order for sub-protocols. Therefore, we need to pass the complete list of requested sub-protocols to the user servlet for it to pick one
b) The server is free to not select one of the sub-protocols. This is not an error state.

Hopefully, I'll add the protocol handling stuff later this evening.
Comment 38 Mark Thomas 2012-02-16 23:30:35 UTC
I am extremely reluctant to apply the current fragmentation patch. It relies on buffering individual fragments and - given the maximum fragment size - that is simply not going to scale. This is why the current API is built on streams and does not buffer unless the message based API is used.

I'd like to fully explore the possibility of supporting fragments without using buffering before starting down that path.

The patch includes other fixes and refactorings. I'll take a look at those next - probably in the next few days.
Comment 39 Mark Thomas 2012-02-19 16:35:14 UTC
Short update for folks not following the dev list.
- added fragmented packet support
- added *very* basic test client
Comment 40 Mark Thomas 2012-02-23 12:42:40 UTC
The bulk of the implementation is complete in trunk and should be at a stage where folks can take it out for a spin. Keep in mind it is far from final. One of things where feedback is required is the API so the sooner you start using it, the greater your chances of influencing the API. See the dev list for a more detailed TODO list if you want to help out.
Comment 41 Johno Crawford 2012-02-28 11:43:52 UTC
Created attachment 28396 [details]
Autobahn fail threaddump

Thought it might be interesting to try out Autobahn with latest trunk (r1294541) on a couple of different platforms to see how things go. I noticed when running the testsuite there was some blocking, eventually I terminated the process.. have you experienced this behaviour Mark?

http://www.hellface.com/Autobahnr1294110.zip

SVN Revision: 1294541

Windows 7 Ultimate 64-bit (6.1, Build 7601) Service Pack 1
java version "1.6.0_30"
Java(TM) SE Runtime Environment (build 1.6.0_30-b12)
Java HotSpot(TM) 64-Bit Server VM (build 20.5-b03, mixed mode)

Ubuntu 10.04.1 LTS
Linux ubuntu-dev 2.6.32-24-generic #42-Ubuntu SMP Fri Aug 20 14:24:04 UTC 2010 i686 GNU/Linux
java version "1.6.0_21"
Java(TM) SE Runtime Environment (build 1.6.0_21-b06)
Java HotSpot(TM) Server VM (build 17.0-b16, mixed mode)
Comment 42 Mark Thomas 2012-02-28 11:56:35 UTC
(In reply to comment #41)
> Thought it might be interesting to try out Autobahn with latest trunk
> (r1294541) on a couple of different platforms to see how things go.

Try reading the dev list.

Autobahn passes fully for all three connectors (BIO, NIO and APR) on Windows and Linux. Things to note:
- you'll need to use the message API, not the stream API
- you'll need to increase the default buffer size to > 16M to handle the tests with larger payloads
- python on Windows has a serious performance issue when sending lots of small packets. Tests that take ~10s on Linux can take 4-5 minutes on Windows.

All of the above has been reported on the dev list in the last week or so.
Comment 43 Johno Crawford 2012-02-28 17:29:47 UTC
(In reply to comment #42)
> (In reply to comment #41)
> > Thought it might be interesting to try out Autobahn with latest trunk
> > (r1294541) on a couple of different platforms to see how things go.
> 
> Try reading the dev list.
> 
> Autobahn passes fully for all three connectors (BIO, NIO and APR) on Windows
> and Linux. Things to note:
> - you'll need to use the message API, not the stream API
> - you'll need to increase the default buffer size to > 16M to handle the tests
> with larger payloads
> - python on Windows has a serious performance issue when sending lots of small
> packets. Tests that take ~10s on Linux can take 4-5 minutes on Windows.
> 
> All of the above has been reported on the dev list in the last week or so.

Thanks for your fast response Mark.  I try and keep up with all of the websocket talk on the dev list, however, I may have missed a few messages.  I do recall some talk about buffer sizes but not seeing anything related to using message API vs stream API.  I am planning to look at some of the remaining TODOs which is why I setup Autobahn, additionally I thought it would be beneficial testing Tomcat websockets on different environments to ensure expected behaviour.
Comment 44 Johno Crawford 2012-02-29 15:12:41 UTC
Created attachment 28407 [details]
Introduce status code constants

Please find attached proposal of introducing status code constants instead of using magic numbers everywhere, also contains some typo fixes.
Comment 45 Mark Thomas 2012-02-29 20:56:34 UTC
(In reply to comment #44)
> Please find attached proposal of introducing status code constants instead of
> using magic numbers everywhere, also contains some typo fixes.

Thanks. Patch applied with a few variations for consistency with the rest of the Tomcat code base.
Comment 46 Johno Crawford 2012-03-01 19:13:34 UTC
Created attachment 28408 [details]
Overrides for connection open and close

I am currently working on an example for websockets. Please consider attached attachment for notifications when connection is opened / closed.
Comment 47 Johno Crawford 2012-03-02 01:57:32 UTC
(In reply to comment #46)
> Created attachment 28408 [details]
> Overrides for connection open and close
> 
> I am currently working on an example for websockets. Please consider attached
> attachment for notifications when connection is opened / closed.

onOpen is not quite in the right place as write can be called before the sockets / streams have been set (callback happens in setUpgradeOutbound..). Can we modify the UpgradeInbound interface to include onUpgradeComplete?
Comment 48 Johno Crawford 2012-03-02 12:53:06 UTC
Created attachment 28410 [details]
Improved echo example

Please find attached proposal for improved echo example. Still working on a much more exciting example by the way!
Comment 49 Johno Crawford 2012-03-02 15:39:30 UTC
Created attachment 28411 [details]
Overrides for connection open and close

Please find attached revised connection open and close overrides which should not cause NPE if write happens just after onOpen is called.
Comment 50 Mark Thomas 2012-03-02 19:10:05 UTC
(In reply to comment #49)
> Created attachment 28411 [details]
> Overrides for connection open and close

Patch applied (with some minor tweaks). Many thanks.

I can see this evolving over time. We may need to change this to an approach (particularly for close) where the method intended to be overridden actually does the work and the developer has to call super.onClose() at an appropriate point. Alternatively, the can not call super.close() and take responsibility for sending the close message themselves. However, we can hold off on that refactoring until someone indicates they need it.
Comment 51 Mark Thomas 2012-03-02 19:15:40 UTC
(In reply to comment #48)
> Created attachment 28410 [details]
> Improved echo example

Patch applied. Many thanks.
Comment 52 Johno Crawford 2012-03-04 12:12:58 UTC
Created attachment 28415 [details]
Connection close not called on IOE

I am able to continually produce this using Chrome 17.0.963.56 m and just closing the tab whilst the websocket connection is active. Please find attached fix proposal.
Comment 53 Mark Thomas 2012-03-04 14:25:41 UTC
(In reply to comment #52)
> Created attachment 28415 [details]
> Connection close not called on IOE

I fixed this slightly differently in trunk, mainly because I wanted to avoid the nested try/catch blocks. Let us know if there is still an issue.
Comment 54 Johno Crawford 2012-03-05 00:48:11 UTC
Created attachment 28416 [details]
Multiplayer snake example

Please find attached patch for simple multiplayer snake example.
Comment 55 Johno Crawford 2012-03-05 00:51:03 UTC
(In reply to comment #53)
> (In reply to comment #52)
> > Created attachment 28415 [details]
> > Connection close not called on IOE
> 
> I fixed this slightly differently in trunk, mainly because I wanted to avoid
> the nested try/catch blocks. Let us know if there is still an issue.

Works as expected, thanks!
Comment 56 Johno Crawford 2012-03-05 09:26:31 UTC
Created attachment 28420 [details]
Multiplayer snake example
Comment 57 Mark Thomas 2012-03-06 15:15:27 UTC
(In reply to comment #56)
> Created attachment 28420 [details]
> Multiplayer snake example

Brilliant. Simply brilliant.

I did clean up a few warnings and reduced some line lengths. I also removed the author tags since we no longer add those. You will get credit in the commit message and the change log.

The only niggle I could find is that a snake can cross over itself. Is that meant to be allowed?
Comment 58 Johno Crawford 2012-03-06 21:54:46 UTC
Created attachment 28426 [details]
Multiplayer snake collision fix

Thanks for taking the time to fix the warnings, formatting and taking in the patch Mark. Please find attached patch which should fix the collision problems.
Comment 59 Mark Thomas 2012-03-06 22:21:36 UTC
(In reply to comment #58)
> Created attachment 28426 [details]
> Multiplayer snake collision fix
> 
> Thanks for taking the time to fix the warnings, formatting and taking in the
> patch Mark. Please find attached patch which should fix the collision problems.

Many thanks for such a speedy response. I wasn't even sure it was a bug.

As an example, this really is fantastic. Your efforts are very much appreciated.
Comment 60 Johno Crawford 2012-03-12 21:59:38 UTC
Created attachment 28457 [details]
Chat example collision fix and cleanup

Please find attached patch for simple chat example, more fixes to collision, repackaging of echo and some cleanup.
Comment 61 Mark Thomas 2012-03-15 23:58:36 UTC
WebSocket support has now been back-ported to 7.0.x and will be included in 7.0.27 onwards.

To prevent what is already a fairly lengthy issue becoming longer, bugs in the implementation, requests for new features, patches etc. should be provided via new bugzilla issues rather than re-opening this one.
Comment 62 Konstantin Kolinko 2012-03-16 00:54:29 UTC
Just a note:
Backport of WebSockets support to Tomcat 6 is tracked separately as bug #52918
Comment 63 Julian Hall 2013-04-23 08:51:48 UTC
Query - now JSR 356 has been finalised with a substantially different API to the one you have implemented here, will this API continue to be supported, or will it be removed in a near-future release?
Comment 64 Mark Thomas 2013-04-23 08:55:55 UTC
The plan is:
1. Add the JSR 356 implementation to Tomcat 7
2. Deprecate Tomcat's proprietary API in Tomcat 7 (it will still be supported for Tomcat 7)
3. Remove Tomcat's proprietary API in Tomcat 8 onwards.

3 has already been done. 1 & 2 will follow once Tomcat 8's JSR356 implementation has stabilised.
Comment 65 Konstantin Kolinko 2014-09-23 10:34:33 UTC
(In reply to Mark Thomas from comment #64)
> The plan is:
> 1. Add the JSR 356 implementation to Tomcat 7
> 2. Deprecate Tomcat's proprietary API in Tomcat 7 (it will still be
> supported for Tomcat 7)
> 3. Remove Tomcat's proprietary API in Tomcat 8 onwards.
> 
> 3 has already been done. 1 & 2 will follow once Tomcat 8's JSR356
> implementation has stabilised.

For a historical record,
backport of JSR 356 implementation to Tomcat 7
and deprecation of Tomcat's proprietary API was done in Tomcat 7.0.43, released in 7.0.47. (Versions 7.0.43-.46 did not pass release vote).

To make use of JSR 356 in Tomcat 7, one has to run it with Java 7 or later.