On 05/23/2013 12:57 AM, Greg Wilkins wrote:
>
>
>
> On 22 May 2013 19:20, Mark Thomas <markt_at_apache.org
> <mailto:markt_at_apache.org>> wrote:
>
> The current sticking point is the non-blocking writes that return a
> Future. Consider the following sequence:
>
> - Container receives WebSocket message
> - Container thread triggers onMessage processing in app
> - app (still on container thread) sends a message in response
> using the
> Future style
> - app calls Future.get()
>
>
>
> I think that the issue here is that you should not call application
> level callback from an async IO callback.
>
> async IO callbacks should not block and calling arbitrary application
> code is possibly going to block with things like databases,
> webservices, let alone request/response IO.
>
>
>
> Other options I have considered:
> 1 Change the non-blocking aspects of the API in Servlet 3.1 to use the
> same style as WebSocket.
>
> 2 Expose the object being used for the one thread per request lock
> through the Servlet API so the WebSocket API can use it rather than
> its own lock to implement blocking. This could enable the
> Servlet API
> to detect when a container thread was blocked and therefore allow
> another thread to do some work on the same request.
>
> 3 Give up and use some the container internals directly.
>
>
>
> Option 4 - to call any code that might block (eg the onMessage
> callback), use AsyncContext.start(Runnable) to obtain a thread to do
> the call into the application.
>
> This allows us to keep 1 container dispatched thread limit, but that
> layers like a JSR356 implementation can request another container
> thread to do independent calls into application code.
>
I mentioned that in an earlier message, but I don't think AsyncContext
is available in upgraded mode ...
Quote of the message:
"Without the clarification, I would say the websocket library has to be
able to access the container thread pool at some point to tell it to run
its code (to cover the fact that onMessage can do blocking operations).
The AsyncContext.start(Runnable) functionality could have been used
there, but I don't think it is available."
And:
"So the possible solutions (in my personal order of preference) are:
- Adding autoblocking (it is vaguely mentioned as being "illegal" right
now; if this is not mentioned anymore, the API semantics apply, and they
should block): it is easy, sneaky and solves the problem, but the
implementation changes are also significant
- Allowing (actually: requiring) concurrent read/write events: it sounds
simple but it leads to more issues, and it should really only be done
once upgraded, but it could still lead to implementation problems (needs
investigation)
- Add a WebConnection.start(Runnable): it makes the API clunky and
people would wonder why it is there"
Note: I made a mistake then, since [my take on] autoblocking doesn't
solve all the websockets use cases.
Actually, it could be nice to have both items (concurrent read/write in
upgrade mode + WebConnection.start(Runnable) to allow tapping into the
container pool).
Rémy